From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Wed, 20 Sep 2017 07:00:48 GMT
Subject: Re: [PATCH v2] for_each_string_list_item: avoid undefined behavior for empty list
Message-ID: <124b960d-a863-18ef-54a2-b170036dfca2@alum.mit.edu>
In-Reply-To: <20170920052705.GC126984@aiede.mtv.corp.google.com>

On 09/20/2017 07:27 AM, Jonathan Nieder wrote:
> From: Michael Haggerty <mhagger@alum.mit.edu>
> 
> If you pass a newly initialized or newly cleared `string_list` to
> `for_each_string_list_item()`, then the latter does
> 
>     for (
>             item = (list)->items; /* NULL */
>             item < (list)->items + (list)->nr; /* NULL + 0 */
>             ++item)
> 
> Even though this probably works almost everywhere, it is undefined
> behavior, and it could plausibly cause highly-optimizing compilers to
> misbehave.  C99 section 6.5.6 paragraph 8 explains:
> 
>     If both the pointer operand and the result point to elements
>     of the same array object, or one past the last element of the
>     array object, the evaluation shall not produce an overflow;
>     otherwise, the behavior is undefined.
> 
> and (6.3.2.3.3) a null pointer does not point to anything.
> 
> Guard the loop with a NULL check to make the intent crystal clear to
> even the most pedantic compiler.  A suitably clever compiler could let
> the NULL check only run in the first iteration, but regardless, this
> overhead is likely to be dwarfed by the work to be done on each item.
> 
> This problem was noticed by Coverity.
> 
> [jn: using a NULL check instead of a placeholder empty list;
>  fleshed out the commit message based on mailing list discussion]

Thanks for taking this over. This version LGTM.

> [...]
Michael