[libvirt] [PATCH 1/2] error: drop old-style error reporting

Eric Blake eblake at redhat.com
Tue Nov 29 13:53:44 UTC 2011


While we still don't want to enable gcc's new -Wformat-literal
warning, I found a rather easy case where the warning could be
reduced, by getting rid of obsolete error-reporting practices.
This is the last place where we were passing the (unused) net
and conn arguments for constructing an error.

* src/util/virterror_internal.h (virErrorMsg): Delete prototype.
(virReportError): Delete macro.
* src/util/virterror.c (virErrorMsg): Make static.
* src/libvirt_private.syms (virterror_internal.h): Drop export.
* src/util/conf.c (virConfError): Convert to macro.
(virConfErrorHelper): New function, and adjust error calls.
* src/xen/xen_hypervisor.c (virXenErrorFunc): Delete.
(xenHypervisorGetSchedulerType)
(xenHypervisorGetSchedulerParameters)
(xenHypervisorSetSchedulerParameters)
(xenHypervisorDomainBlockStats)
(xenHypervisorDomainInterfaceStats)
(xenHypervisorDomainGetOSType)
(xenHypervisorNodeGetCellsFreeMemory, xenHypervisorGetVcpus):
Update callers.
---
 src/libvirt_private.syms      |    1 -
 src/util/conf.c               |   22 ++---
 src/util/virterror.c          |    2 +-
 src/util/virterror_internal.h |    8 --
 src/xen/xen_hypervisor.c      |  170 +++++++++++++++++------------------------
 5 files changed, 79 insertions(+), 124 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9f2a224..74412b0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1370,7 +1370,6 @@ virSocketAddrSetPort;

 # virterror_internal.h
 virDispatchError;
-virErrorMsg;
 virRaiseErrorFull;
 virReportErrorHelper;
 virReportOOMErrorFull;
diff --git a/src/util/conf.c b/src/util/conf.c
index c8dcc7f..92bc84a 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -89,27 +89,23 @@ struct _virConf {
  *
  * Handle an error at the xend daemon interface
  */
+#define virConfError(ctxt, error, info) \
+    virConfErrorHelper(__FILE__, __FUNCTION__, __LINE__, ctxt, error, info)
 static void
-virConfError(virConfParserCtxtPtr ctxt,
-             virErrorNumber error, const char *info)
+virConfErrorHelper(const char *file, const char *func, size_t line,
+                   virConfParserCtxtPtr ctxt,
+                   virErrorNumber error, const char *info)
 {
-    const char *format;
-
     if (error == VIR_ERR_OK)
         return;

     /* Construct the string 'filename:line: info' if we have that. */
     if (ctxt && ctxt->filename) {
-        virRaiseError(NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR,
-                        info, ctxt->filename, NULL,
-                        ctxt->line, 0,
-                        "%s:%d: %s", ctxt->filename, ctxt->line, info);
+        virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+                             _("%s:%d: %s"), ctxt->filename, ctxt->line, info);
     } else {
-        format = virErrorMsg(error, info);
-        virRaiseError(NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR,
-                        info, NULL, NULL,
-                        ctxt ? ctxt->line : 0, 0,
-                        format, info);
+        virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+                             "%s", info);
     }
 }

diff --git a/src/util/virterror.c b/src/util/virterror.c
index 44a276a..e2a0f3a 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -755,7 +755,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
  *
  * Returns the constant string associated to @error
  */
-const char *
+static const char *
 virErrorMsg(virErrorNumber error, const char *info)
 {
     const char *errmsg = NULL;
diff --git a/src/util/virterror_internal.h b/src/util/virterror_internal.h
index d61ea0d..b8cb279 100644
--- a/src/util/virterror_internal.h
+++ b/src/util/virterror_internal.h
@@ -47,14 +47,6 @@ void virRaiseErrorFull(const char *filename,
                        const char *fmt, ...)
     ATTRIBUTE_FMT_PRINTF(12, 13);

-/* Includes 'dom' and 'net' for compatbility, but they're ignored */
-# define virRaiseError(dom, net, domain, code, level,              \
-                      str1, str2, str3, int1, int2, msg, ...)           \
-    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,           \
-                      domain, code, level, str1, str2, str3, int1, int2, \
-                      msg, __VA_ARGS__)
-
-const char *virErrorMsg(virErrorNumber error, const char *info);
 void virReportErrorHelper(int domcode, int errcode,
                           const char *filename,
                           const char *funcname,
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 870bc4f..ef64177 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -827,40 +827,6 @@ struct xenUnifiedDriver xenHypervisorDriver = {
                                  __FUNCTION__, __LINE__, __VA_ARGS__)

 /**
- * virXenErrorFunc:
- * @error: the error number
- * @func: the function failing
- * @info: extra information string
- * @value: extra information number
- *
- * Handle an error at the xend daemon interface
- */
-static void
-virXenErrorFunc(virErrorNumber error, const char *func, const char *info,
-                int value)
-{
-    char fullinfo[1000];
-    const char *errmsg;
-
-    if ((error == VIR_ERR_OK) || (in_init != 0))
-        return;
-
-
-    errmsg =virErrorMsg(error, info);
-    if (func != NULL) {
-        snprintf(fullinfo, 999, "%s: %s", func, info);
-        fullinfo[999] = 0;
-        virRaiseError(NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
-                        errmsg, fullinfo, NULL, value, 0, errmsg, fullinfo,
-                        value);
-    } else {
-        virRaiseError(NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
-                        errmsg, info, NULL, value, 0, errmsg, info,
-                        value);
-    }
-}
-
-/**
  * xenHypervisorDoV0Op:
  * @handle: the handle to the Xen hypervisor
  * @op: pointer to the hypervisor operation structure
@@ -1142,15 +1108,15 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
     xenUnifiedPrivatePtr priv;

     if (domain->conn == NULL) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "domain or conn is NULL", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("domain or conn is NULL"));
         return NULL;
     }

     priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
     if (priv->handle < 0) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "priv->handle invalid", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("priv->handle invalid"));
         return NULL;
     }
     if (domain->id < 0) {
@@ -1165,8 +1131,8 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
      * TODO: check on Xen 3.0.3
      */
     if (hv_versions.dom_interface < 5) {
-        virXenErrorFunc(VIR_ERR_NO_XEN, __FUNCTION__,
-                        "unsupported in dom interface < 5", 0);
+        virXenError(VIR_ERR_NO_XEN, "%s",
+                    _("unsupported in dom interface < 5"));
         return NULL;
     }

@@ -1223,15 +1189,15 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
     xenUnifiedPrivatePtr priv;

     if (domain->conn == NULL) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "domain or conn is NULL", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("domain or conn is NULL"));
         return -1;
     }

     priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
     if (priv->handle < 0) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "priv->handle invalid", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("priv->handle invalid"));
         return -1;
     }
     if (domain->id < 0) {
@@ -1246,8 +1212,8 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
      * TODO: check on Xen 3.0.3
      */
     if (hv_versions.dom_interface < 5) {
-        virXenErrorFunc(VIR_ERR_NO_XEN, __FUNCTION__,
-                        "unsupported in dom interface < 5", 0);
+        virXenError(VIR_ERR_NO_XEN, "%s",
+                    _("unsupported in dom interface < 5"));
         return -1;
     }

@@ -1309,8 +1275,9 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
                     *nparams = XEN_SCHED_CRED_NPARAM;
                 break;
             default:
-                virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "Unknown scheduler", op_sys.u.getschedulerid.sched_id);
+                virXenError(VIR_ERR_INVALID_ARG,
+                            _("Unknown scheduler %d"),
+                            op_sys.u.getschedulerid.sched_id);
                 return -1;
         }
     }
@@ -1337,8 +1304,8 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
     char buf[256];

     if (domain->conn == NULL) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "domain or conn is NULL", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("domain or conn is NULL"));
         return -1;
     }

@@ -1349,8 +1316,8 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,

     priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
     if (priv->handle < 0) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "priv->handle invalid", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("priv->handle invalid"));
         return -1;
     }
     if (domain->id < 0) {
@@ -1365,8 +1332,8 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
      * TODO: check on Xen 3.0.3
      */
     if (hv_versions.dom_interface < 5) {
-        virXenErrorFunc(VIR_ERR_NO_XEN, __FUNCTION__,
-                        "unsupported in dom interface < 5", 0);
+        virXenError(VIR_ERR_NO_XEN, "%s",
+                    _("unsupported in dom interface < 5"));
         return -1;
     }

@@ -1405,8 +1372,9 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
                     params[i].type == VIR_TYPED_PARAM_UINT) {
                     val = params[i].value.ui;
                     if ((val < 1) || (val > USHRT_MAX)) {
-                        snprintf(buf, sizeof(buf), _("Credit scheduler weight parameter (%d) is out of range (1-65535)"), val);
-                        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__, buf, val);
+                        virXenError(VIR_ERR_INVALID_ARG,
+                                    _("Credit scheduler weight parameter (%d) "
+                                      "is out of range (1-65535)"), val);
                         return(-1);
                     }
                     op_dom.u.getschedinfo.u.credit.weight = val;
@@ -1414,15 +1382,16 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
                     params[i].type == VIR_TYPED_PARAM_UINT) {
                     val = params[i].value.ui;
                     if (val >= USHRT_MAX) {
-                        snprintf(buf, sizeof(buf), _("Credit scheduler cap parameter (%d) is out of range (0-65534)"), val);
-                        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__, buf, val);
+                        virXenError(VIR_ERR_INVALID_ARG,
+                                    _("Credit scheduler cap parameter (%d) is "
+                                      "out of range (0-65534)"), val);
                         return(-1);
                     }
                     op_dom.u.getschedinfo.u.credit.cap = val;
                 } else {
-                    virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                                    "Credit scheduler accepts 'cap' and 'weight' integer parameters",
-                                    0);
+                    virXenError(VIR_ERR_INVALID_ARG, "%s",
+                                _("Credit scheduler accepts 'cap' and "
+                                  "'weight' integer parameters"));
                     return(-1);
                 }
             }
@@ -1433,8 +1402,9 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
             break;
         }
         default:
-            virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "Unknown scheduler", op_sys.u.getschedulerid.sched_id);
+            virXenError(VIR_ERR_INVALID_ARG,
+                        _("Unknown scheduler %d"),
+                        op_sys.u.getschedulerid.sched_id);
             return -1;
         }
     }
@@ -1459,9 +1429,8 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
     xenUnifiedUnlock(priv);
     return ret;
 #else
-    virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
-                    "block statistics not supported on this platform",
-                    dom->id);
+    virXenError(VIR_ERR_OPERATION_INVALID, "%s",
+                _("block statistics not supported on this platform"));
     return -1;
 #endif
 }
@@ -1484,21 +1453,21 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
     /* Verify that the vif requested is one belonging to the current
      * domain.
      */
-    if (sscanf (path, "vif%d.%d", &rqdomid, &device) != 2) {
-        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "invalid path, should be vif<domid>.<n>.", 0);
+    if (sscanf(path, "vif%d.%d", &rqdomid, &device) != 2) {
+        virXenError(VIR_ERR_INVALID_ARG, "%s",
+                    _("invalid path, should be vif<domid>.<n>."));
         return -1;
     }
     if (rqdomid != dom->id) {
-        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "invalid path, vif<domid> should match this domain ID", 0);
+        virXenError(VIR_ERR_INVALID_ARG, "%s",
+                    _("invalid path, vif<domid> should match this domain ID"));
         return -1;
     }

     return linuxDomainInterfaceStats(path, stats);
 #else
-    virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
-                    "/proc/net/dev: Interface not found", 0);
+    virXenError(VIR_ERR_OPERATION_INVALID, "%s",
+                _("/proc/net/dev: Interface not found"));
     return -1;
 #endif
 }
@@ -2897,30 +2866,30 @@ xenHypervisorDomainGetOSType (virDomainPtr dom)

     priv = (xenUnifiedPrivatePtr) dom->conn->privateData;
     if (priv->handle < 0) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        _("domain shut off or invalid"), 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("domain shut off or invalid"));
         return (NULL);
     }

     /* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/
     if (hv_versions.hypervisor < 2 ||
         hv_versions.dom_interface < 4) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        _("unsupported in dom interface < 4"), 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("unsupported in dom interface < 4"));
         return (NULL);
     }

     XEN_GETDOMAININFO_CLEAR(dominfo);

     if (virXen_getdomaininfo(priv->handle, dom->id, &dominfo) < 0) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        _("cannot get domain details"), 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("cannot get domain details"));
         return (NULL);
     }

     if (XEN_GETDOMAININFO_DOMAIN(dominfo) != dom->id) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        _("cannot get domain details"), 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("cannot get domain details"));
         return (NULL);
     }

@@ -3319,22 +3288,21 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
     xenUnifiedPrivatePtr priv;

     if (conn == NULL) {
-        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "invalid argument", 0);
+        virXenError(VIR_ERR_INVALID_ARG, "%s", _("invalid argument"));
         return -1;
     }

     priv = conn->privateData;

     if (priv->nbNodeCells < 0) {
-        virXenErrorFunc(VIR_ERR_XEN_CALL, __FUNCTION__,
-                        "cannot determine actual number of cells",0);
+        virXenError(VIR_ERR_XEN_CALL, "%s",
+                    _("cannot determine actual number of cells"));
         return(-1);
     }

     if ((maxCells < 1) || (startCell >= priv->nbNodeCells)) {
-        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "invalid argument", 0);
+        virXenError(VIR_ERR_INVALID_ARG, "%s",
+                    _("invalid argument"));
         return -1;
     }

@@ -3342,14 +3310,14 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
      * Support only hv_versions.sys_interface >=4
      */
     if (hv_versions.sys_interface < SYS_IFACE_MIN_VERS_NUMA) {
-        virXenErrorFunc(VIR_ERR_XEN_CALL, __FUNCTION__,
-                        "unsupported in sys interface < 4", 0);
+        virXenError(VIR_ERR_XEN_CALL, "%s",
+                    _("unsupported in sys interface < 4"));
         return -1;
     }

     if (priv->handle < 0) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        "priv->handle invalid", 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("priv->handle invalid"));
         return -1;
     }

@@ -3590,13 +3558,13 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
     if (priv->handle < 0 || (domain->id < 0) ||
         (info == NULL) || (maxinfo < 1) ||
         (sizeof(cpumap_t) & 7)) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        _("domain shut off or invalid"), 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("domain shut off or invalid"));
         return (-1);
     }
     if ((cpumaps != NULL) && (maplen < 1)) {
-        virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,
-                        "invalid argument", 0);
+        virXenError(VIR_ERR_INVALID_ARG, "%s",
+                    _("invalid argument"));
         return -1;
     }
     /* first get the number of virtual CPUs in this domain */
@@ -3605,8 +3573,8 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
                                &dominfo);

     if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != domain->id)) {
-        virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                        _("cannot get domain details"), 0);
+        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                    _("cannot get domain details"));
         return (-1);
     }
     nbinfo = XEN_GETDOMAININFO_CPUCOUNT(dominfo) + 1;
@@ -3622,16 +3590,16 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
                                       (unsigned char *)VIR_GET_CPUMAP(cpumaps, maplen, i),
                                       maplen);
             if (ret < 0) {
-                virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                                _("cannot get VCPUs info"), 0);
+                virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                            _("cannot get VCPUs info"));
                 return(-1);
             }
         } else {
             ret = virXen_getvcpusinfo(priv->handle, domain->id, i,
                                       ipt, NULL, 0);
             if (ret < 0) {
-                virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
-                                _("cannot get VCPUs info"), 0);
+                virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
+                            _("cannot get VCPUs info"));
                 return(-1);
             }
         }
-- 
1.7.7.3




More information about the libvir-list mailing list