Re: [PATCH] Fix undefined behaviour inconsistent for strtok

[Wilco Dijkstra <Wilco.Dijkstra@arm.com>]

Hi,

+  if ((s == NULL) && ((s = olds) == NULL))
+    return NULL;

What is the benefit of this given:

  if (s == NULL)
    /* This token finishes the string.  */
    olds = __rawmemchr (token, '\0');

So in the current implementation 'olds' can only ever be NULL at the
very first call to strtok.

To avoid doing unnecessary work at the end of a string and avoid
use after free or other memory errors, this would be much better:

  if (s == NULL)
    /* This token finishes the string.  */
    olds = NULL;

Setting it to a NULL pointer (and not checking for it on entry) causes a crash
so any bug is found immediately rather than potentially staying latent when 
returning NULL. The goal should be to make bugs obvious, not trying to hide them.

Btw strtok_r has more potential issues, the reference to the previous string
may be a NULL pointer, and the pointer it contains may not be initialized at
all, so it's not useful to test either.

Wilco