On 04/04/2010 11:36 AM, Matthias Bolte wrote:
--- src/libvirt.c | 1577 +++++++++++++++++++++++++-------------------------------- 1 files changed, 697 insertions(+), 880 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index fb683c0..788b943 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -428,38 +428,19 @@ DllMain (HINSTANCE instance ATTRIBUTE_UNUSED, } #endif
- -/** - * virLibConnError: - * @conn: the connection if available - * @error: the error number - * @info: extra information string - * - * Handle an error at the connection level - */ -static void -virLibConnError(virConnectPtr conn, virErrorNumber error, const char *info) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = virErrorMsg(error, info); - virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info); -} +#define virLibConnError(code, ...) \ + virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__)
Any reason you are deleting, rather than updating, the documentation? It can still be useful to document macros.
@@ -697,12 +499,12 @@ virRegisterNetworkDriver(virNetworkDriverPtr driver) return -1;
if (driver == NULL) { - virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, "%s", __FUNCTION__);
Did gcc seriously warn on this? __FUNCTION__ cannot contain %, and is not translated. Maybe it's worth a QoI bug report to gcc, because I see no reason why __FUNCTION__ should not be directly usable as a format string. Then again, why are we passing __FUNCTION__ ourselves? That seems awkward, especially given that you just defined virLibConnError to pass __FUNCTION__ to virReportErrorHelper, so now we've ended up using __FUNCTION__ twice. Isn't the whole point of going through a macro to make it easier to apply __FUNCTION__ automatically without having to think about it at every client site?
@@ -1217,8 +1018,8 @@ do_open (const char *name, (res == VIR_DRV_OPEN_ERROR ? "ERROR" : "unknown status"))); if (res == VIR_DRV_OPEN_ERROR) { if (STREQ(virNetworkDriverTab[i]->name, "remote")) { - virLibConnWarning (NULL, VIR_WAR_NO_NETWORK, - "Is the daemon running ?"); + virLibConnWarning(VIR_WAR_NO_NETWORK, + "Is the daemon running ?");
Is virLibConnWarning something that deserves translation? And the space before ? looks weird, as long as we are touching this line. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org