Re: [libvirt] [PATCH] remote_internal.c: don't dereference a NULL "conn"

Daniel Veillard wrote: > On Wed, Sep 02, 2009 at 11:58:07AM +0200, Jim Meyering wrote: >> Here's the test just before the else-if in the patch below: >> >> if (conn && >> conn->driver && >> STREQ (conn->driver->name, "remote")) { >> >> So, in the else-branch, "conn" is guaranteed to be NULL. >> And dereferenced. >> >> This may be only a theoretical risk, but if so, >> the test of "conn" above should be changed to an assertion, >> and/or the parameter should get the nonnull attribute. > > I'm fine with your patch, so that even if code around changes > having a gard is IMHO a good thing Thanks for the quick review. Daniel Berrange said he'd prefer the nonnull approach I mentioned above, so I've just adjusted (caveat, still not tested or even compiled) >From 375bbb2d85fdf708858d726b6c7682e40dc07c2c Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Wed, 2 Sep 2009 12:20:32 +0200 Subject: [PATCH 1/2] infra: define ATTRIBUTE_NONNULL to mark non-NULL parameters * src/internal.h (ATTRIBUTE_NONNULL): Define.

>From 61dc45fcae8b17d9f619563d427f1bc74f44d553 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Wed, 2 Sep 2009 12:22:14 +0200 Subject: [PATCH 2/2] remote_internal.c: appease clang * src/remote_internal.c (remoteNetworkOpen): Mark "conn" parameter as non-NULL. Remove now-unnecessary "conn == NULL" test. (remoteDevMonOpen): Likewise.