[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] warnings: address -Wnull-dereference in reader.c

From: Paul Eggert
Subject: Re: [PATCH] warnings: address -Wnull-dereference in reader.c
Date: Wed, 15 Aug 2018 09:34:05 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

Akim Demaille wrote:
will C stop requiring
void some day?

It's not likely any time soon, since K&R-style function definitions are still allowed in C11 and even if they're removed it'll likely require two iterations of the standard to do it.

+  argmatch assert assume

'assume'? There is no assume module in gnulib; the 'assume' macro is defined by verify.h, in the 'verify' module. If reader.c uses 'assume' it should include verify.h.

 static symbol *
-find_start_symbol ()
+find_start_symbol (void)
   symbol_list *res = grammar;
   for (;
-       res != NULL && symbol_is_dummy (res->content.sym);
+       res && symbol_is_dummy (res->content.sym);
        res = res->next)
       for (res = res->next;
-           res != NULL && res->content.sym != NULL;
+           res && res->content.sym;
            res = res->next)
-      aver (res != NULL);
+      assume (res);
-  aver (res != NULL);
+  assume (res);
   return res->content.sym;

This doesn't look right, as 'assume (X)' is an instruction from the programmer to the compiler, saying "I know that X is nonzero even though you don't, so compile the code assuming that X is nonzero even if the compiled code would be incorrect if X were zero." And yet here, you have a for loop that explicitly exits the loop if res is null, and then says 'assume (res)'. Either the test for res is wrong, or the assume is wrong.

If you already know that the start symbol is in the grammar, then write the code that way; there is no need to test whether res is nonzero. That way, you will probably be able to pacify GCC without using 'assume' at all.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]