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)
From 375bbb2d85fdf708858d726b6c7682e40dc07c2c Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> 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@redhat.com> 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.
Note that this patch now address two reported NULL-deref problems, not just one, like the original.