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.