Re: [libvirt] [PATCH 6/7] xend_internal.c: assure clang that we do not dereference NULL
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 323 bytes)