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.
Does __attribute__((__nonnull__())) really cover the case we're
concerned about here? As I understand it, it tells the compiler
1) if the caller obviously passes NULL, emit a warning
2) assume that the arguments are non-NULL and optimize away
In other words it will do nothing to prevent a null dereference and
will even make it more likely since non-NULL checks will be skipped.
For example:
test.c:
--------
#include <stdio.h>
#include <stdlib.h>
void foo(int *a) {
if (a) printf("%d\n", *a);
else printf("(null)\n");
}
__attribute__((__nonnull__ (1)))
void bar(int *a) {
if (a) printf("%d\n", *a);
else printf("(null)\n");
}
int main(void)
{
foo(malloc(1000000000000ULL));
bar(malloc(1000000000000ULL));
return 0;
}
--------
$ gcc -O2 -Wall -Werror -o test test.c
$ ./test
(null)
Segmentation fault
$
-jim
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list