[libvirt] [PATCH 6/7] xend_internal.c: assure clang that we do not dereference NULL

Eric Blake eblake at redhat.com
Wed Apr 14 17:39:12 UTC 2010

On 04/14/2010 11:29 AM, Jim Meyering wrote:
>>>      } else if (STREQ(type, "tcp")) {
>>> +        sa_assert (value);
>>>          const char *offset = strchr(value, ':');
>> This introduces an expression before a declaration, even when sa_assert
>> is empty.  I know that we already rely on several other C99 features
>> (like __VA_ARGS__ from the preprocessor), but my understanding has been
>> that we have been trying to stick with C89 declarations first still.
>> Does this need to be refactored accordingly?
> There have been others like this, and no one objected then.
> Prohibiting stmt-after-decl is detrimental.

In re-reading my comment, I can see my position was not clear.  I am
very much in favor of relying on more C99 features, especially
decl-after-stmt.  I'm not opposed to your solution, so much as
questioning whether others are still trying to stick to the C89 rules,
as well as point out that sticking to C89 in this case would be
anachronistic given that we already have rely on other C99 features like
// for comments.

>> Besides, xend_parse_sexp_desc_char already dereferences value at line
>> 1224;
> True, but just after that, it may be set to NULL:
>     if (value[0] == '/') {
>         type = "dev";
>     } else if (STRPREFIX(value, "null")) {
>         type = "null";
>         value = NULL;      <<<<===================

Ahh.  My review was not thorough enough, then.  Still, clang didn't
complain about the first value[0] dereference?

At any rate, your additional explanation, coupled with my second review
of that function, is enough to warrant ACK, once 1/7 is resolved.

Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 323 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100414/4c1b2adf/attachment-0001.sig>

More information about the libvir-list mailing list