Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> *sigh* of course you're right. I should know better than to "fire off a
> quick fix to the mailing list".
>
> I guess the two proposals that are still in the running for rescuing
> this macro are Jonathan's and Gábor's. I have no strong preference
> either way.

If somebody is writing this outisde a macro as a one-shot thing, the
most natural and readable way I would imagine would be

	if (the list is empty)
        	;
	else
		for (each item in the list)
			work on item

I would think.  That "work on item" part may not be a single
expression statement and instead be a compound statement inside a
pair of braces {}.  Making a shorter version, i.e.

	if (!the list is empty)
		for (each item in the list)
			work on item

into a macro probably has syntax issues around cascading if/else
chain, e.g.

	if (condition caller cares about)
		for_each_string_list_item() {
			do this thing
		}
	else
		do something else

would expand to

	if (condition caller cares about)
		if (!the list is empty)
			for (each item in the list) {
				do this thing
			}
	else
		do something else

which is wrong.  But I couldn't think of a way to break the longer
one with the body of the macro in the "else" clause in a similar
way.  An overly helpful compiler might say

	if (condition caller cares about)
		if (the list is empty)
			;
		else
			for (each item in the list) {
				do this thing
			}
	else
		do something else

that it wants a pair of {} around the then-clause of the outer if;
if we can find a way to squelch such warnings only with this
construct that comes from the macro, then this solution may be ideal.

If we cannot do that, then

	for (item = (list)->items; /* could be NULL */
	     (list)->items && item < (list)->items + (list)->nr;
	     item++)
		work on item

may be an obvious way to write it without any such syntax worries,
but I am unclear how a "undefined behaviour" contaminate the code
around it.  My naive reading of the termination condition of the
above is:

	"(list)->items &&" clearly means that (list)->items is not
	NULL in what follows it, i.e. (list->items + (list)->nr
	cannot be a NULL + 0, so we are not allowed to make demon
	fly out of your nose.

but I wonder if this alternative reading is allowed:

	(list)->items is not assigned to in this expression and is
	used in a subexpression "(list)->items + (list)->nr" here;
	for that subexpression not to be "undefined", it cannot be
	NULL, so we can optimize out "do this only (list)->items is
	not NULL" part.

which takes us back to where we started X-<.  So I dunno.

I am hoping that this last one is not allowed and we can use the
"same condition is checked every time we loop" version that hides
the uglyness inside the macro.