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

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