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.
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list