Re: strstr.c:105:3: error: ‘ for’ loo p initial declarations are only allowed in C99 or C11 mode

Corinna Vinschen <[email protected]> Tue, 13 Aug 2019 10:00:54 +0200

Newsgroups	gmane.comp.lib.newlib
Message-ID	<[email protected]>

On Aug 12 20:18, Orlando Arias wrote:
> On 8/12/19 8:05 PM, Howland, Craig D. - US via newlib wrote:
> [snip]
> >>        for (int i = 0; i < ne_len; i++)
> >>        ^
> [snip]
> > 
> > 
> >      A quick fix for you is to do what the error message suggests:  add -std=gnu99 to CFLAGS before you configure Newlib, to have that as a default compiler flag when it is built.
> >     This is probably also the best solution for the real fix in Newlib.  There are other cases of C99 constructs being used, anyway.  (It has been 20 years.  Seems totally reasonable to require it.)
> >                 Craig
> > 
> 
> Greetings,
> 
> There is still a lingering bug in the code above, and the proposed patch
> in [1]. It will trigger undefined behavior due to the comparison of a
> signed type and an unsigned type. If the value of ne_len or similar is
> over INT_MAX, the comparison will always fail and cause an integer
> overflow, resulting in undefined behavior [signed overflow is UB per C
> standard]. The datatype that should be used in these iterations is
> size_t. I do agree however that requiring C99 or newer to compile newlib
> should be a thing.
> 
> Cheers,
> Orlando.
> 
> [1] https://sourceware.org/ml/newlib/2019/msg00469.html
> 

I don't see any such patch in there fixing the undefined behaviour
you're talking about:

@@ -142,6 +145,7 @@ strstr (const char *haystack, const char *needle)
 {
   const unsigned char *hs = (const unsigned char *) haystack;
   const unsigned char *ne = (const unsigned char *) needle;
+  int i;

   /* Handle short needle special cases first.  */
   if (ne[0] == '\0')
@@ -170,7 +174,7 @@ strstr (const char *haystack, const char *needle)

       /* Initialize bad character shift hash table.  */
       memset (shift, ne_len + 1, sizeof (shift));
-      for (int i = 0; i < ne_len; i++)
+      for (i = 0; i < ne_len; i++)
 	shift[ne[i] % sizeof (shift)] = ne_len - i;

       do

These two hunks are just the pre-C99 drop-in replacement.
Also:

    if (__builtin_expect (ne_len < 255, 1))
      [...]

So ne_len is guaranteed to be < 255 in this code snippet.  Am I missing
something?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

signature.asc (application/pgp-signature, 833 B)