On Wed, Jan 29, 2025 at 05:40:26PM +0400, marcandre.lureau@xxxxxxxxxx wrote: > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > Helps avoid/debug a potential SEGV if conn is NULL, since gio will not > set the "gerror" in that case and we will crash later at: > virReportError(VIR_ERR_DBUS_SERVICE, "%s", gerror->message); > > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > --- > src/util/virgdbus.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c > index 71ca3cd43b..5e11a95106 100644 > --- a/src/util/virgdbus.c > +++ b/src/util/virgdbus.c > @@ -213,6 +213,8 @@ virGDBusCallMethod(GDBusConnection *conn, > g_autoptr(GVariant) ret = NULL; > g_autoptr(GError) gerror = NULL; > > + g_return_val_if_fail(conn != NULL, -1); > + > if (error) > memset(error, 0, sizeof(*error)); > > @@ -268,6 +270,8 @@ virGDBusCallMethodWithFD(GDBusConnection *conn, > g_autoptr(GVariant) ret = NULL; > g_autoptr(GError) gerror = NULL; > > + g_return_val_if_fail(conn != NULL, -1); > + We don't use g_return_val_if_fail helpers in libvirt because that introduces inconsistent error reporting behaviour in our APIs. ie all error return paths call virReportError, except for the ones which don't call virReportError. If we expect 'conn' to be non-NULL, then we should at least add ATTRIBUTE_NONNULL in the header though, so static analysis tools validate this. If a runtime check is really needed, then it would have to be a full check with virReportError, but honestly we almost never do that anywhere in libvirt except for a few places where it has been easy to make such a mistake With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
