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

[Jim Meyering]

Daniel P. Berrange wrote:

> On Wed, Sep 02, 2009 at 12:26:43PM +0200, Jim Meyering wrote:
>> 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)
>
> Yeah, for functions where it is expected that the passed in param
> be non-NULL, then annotations are definitely the way togo. This
> lets the compiler/checkers validate the callers instead, avoiding
> the need to clutter the methods with irrelevant NULL checks.

> ACK

Thanks.
I'll post a revised patch once done, and built/tested.
FYI, I see there are very similar ones also in these functions:

  remoteSecretOpen
  remoteStorageOpen
  remoteInterfaceOpen

so far, the changes all look identical.