2010/5/12 Eric Blake <eblake@redhat.com>:
virLibConnError already includes __FUNCTION__ in its output, so we were redundant. Furthermore, clang warns that __FUNCTION__ is not a string literal (at least __FUNCTION__ will never contain %, so it was not a security risk).
* src/datatypes.c: Replace __FUNCTION__ with a descriptive string. --- src/datatypes.c | 36 ++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c index 25962a6..cea8765 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -287,7 +287,7 @@ virUnrefConnect(virConnectPtr conn) { int refs;
if ((!VIR_IS_CONNECT(conn))) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
As Chris pointed out a while ago: https://www.redhat.com/archives/libvir-list/2010-April/msg01241.html There are some error codes that have string representations attached which imply a certain usage style. VIR_ERR_INVALID_ARG is one of those. See virErrorMsg: case VIR_ERR_INVALID_ARG: if (info == NULL) errmsg = _("invalid argument in"); else errmsg = _("invalid argument in %s"); break; This implies that you pass the function name as additional information. But I must admit that I never used VIR_ERR_INVALID_ARG as it is implied, because I wasn't aware of that until recently. The VIR_ERR_XML_ERROR is an even worse example: case VIR_ERR_XML_ERROR: if (info == NULL) errmsg = _("XML description not well formed or invalid"); else errmsg = _("XML description for %s is not well formed or invalid"); break; Unrelated to your patch I suggest that we unify the string representations for error codes to a common style: case VIR_ERR_INVALID_ARG: if (info == NULL) errmsg = _("invalid argument"); else errmsg = _("invalid argument: %s"); break; case VIR_ERR_XML_ERROR: if (info == NULL) errmsg = _("XML description not well formed or invalid"); else errmsg = _("XML description not well formed or invalid: %s"); break; And adapt the callers.
return(-1); } virMutexLock(&conn->lock); @@ -345,7 +345,7 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { virDomainPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/3 of the possibilities to get into this block.
return(NULL); } virMutexLock(&conn->lock); @@ -455,7 +455,7 @@ virUnrefDomain(virDomainPtr domain) { int refs;
if (!VIR_IS_CONNECTED_DOMAIN(domain)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
VIR_IS_CONNECTED_DOMAIN is false if domain isn't a domain, or if it's connection object isn't a connection. Therefore "no connection" is incomplete.
return(-1); } virMutexLock(&domain->conn->lock); @@ -490,7 +490,7 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { virNetworkPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/3 of the possibilities to get into this block.
return(NULL); } virMutexLock(&conn->lock); @@ -592,7 +592,7 @@ virUnrefNetwork(virNetworkPtr network) { int refs;
if (!VIR_IS_CONNECTED_NETWORK(network)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
As with VIR_IS_CONNECTED_DOMAIN above "no connection" is incomplete here.
return(-1); } virMutexLock(&network->conn->lock); @@ -629,7 +629,7 @@ virGetInterface(virConnectPtr conn, const char *name, const char *mac) { virInterfacePtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/2 of the possibilities to get into this block.
return(NULL); }
@@ -774,7 +774,7 @@ virUnrefInterface(virInterfacePtr iface) { int refs;
if (!VIR_IS_CONNECTED_INTERFACE(iface)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
The same as with VIR_IS_CONNECTED_DOMAIN and VIR_IS_CONNECTED_NETWORK above.
return(-1); } virMutexLock(&iface->conn->lock); @@ -810,7 +810,7 @@ virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uui virStoragePoolPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/3 of the possibilities to get into this block.
return(NULL); } virMutexLock(&conn->lock); @@ -913,7 +913,7 @@ virUnrefStoragePool(virStoragePoolPtr pool) { int refs;
if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is incomplete here too.
return(-1); } virMutexLock(&pool->conn->lock); @@ -950,7 +950,7 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c virStorageVolPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (key == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/3 of the possibilities to get into this block.
return(NULL); } virMutexLock(&conn->lock); @@ -1062,7 +1062,7 @@ virUnrefStorageVol(virStorageVolPtr vol) { int refs;
if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
And another incomplete message.
return(-1); } virMutexLock(&vol->conn->lock); @@ -1098,7 +1098,7 @@ virGetNodeDevice(virConnectPtr conn, const char *name) virNodeDevicePtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/2 of the possibilities to get into this block.
return(NULL); } virMutexLock(&conn->lock); @@ -1230,7 +1230,7 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, char uuidstr[VIR_UUID_STRING_BUFLEN];
if (!VIR_IS_CONNECT(conn) || uuid == NULL || usageID == NULL) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/3 of the possibilities to get into this block.
return NULL; } virMutexLock(&conn->lock); @@ -1329,7 +1329,7 @@ virUnrefSecret(virSecretPtr secret) { int refs;
if (!VIR_IS_CONNECTED_SECRET(secret)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
And another incomplete message.
return -1; } virMutexLock(&secret->conn->lock); @@ -1423,7 +1423,7 @@ virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid) virNWFilterPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
"no connection" is only true in 1/3 of the possibilities to get into this block.
return(NULL); } virMutexLock(&conn->lock); @@ -1526,7 +1526,7 @@ virUnrefNWFilter(virNWFilterPtr pool) { int refs;
if (!VIR_IS_CONNECTED_NWFILTER(pool)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection"));
And another incomplete message.
return -1; } virMutexLock(&pool->conn->lock); @@ -1550,7 +1550,7 @@ virGetDomainSnapshot(virDomainPtr domain, const char *name) virDomainSnapshotPtr ret = NULL;
if ((!VIR_IS_DOMAIN(domain)) || (name == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("not a snapshot"));
This block is entered if domain is no domain or name is NULL, therefore "not a snapshot" is a misleading error message here. Matthias