Re: [PATCH 2/3] convert "enum date_mode" into a struct

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

Jeff King <peff@peff.net> writes:

> My assumption was that using the raw "0" is something we would frowned
> upon in new code. There was a single historical instance that I fixed in
> the series, but I wouldn't expect new ones (and actually, that instance
> was "1", which would be caught by the compiler).

That is not the problem.

The code on the side branch may add a new callsite, something like
this:

	show_ident_date(&ident_split, DATE_NORMAL);

based on the current codebase (e.g. 'master' as of today).

The merge goes cleanly, it compiles, even though the new function
signature of show_ident_date(), similar to the updated show_date(),
takes a pointer to a struct where they used to take DATE_$format
constants.

And that is because DATE_NORMAL is defined to be 0; we can claim
that the compiler is being stupid to take one of the enum
date_mode_type values that happens to be 0 and misinterpret it as
the program wanted to pass a NULL pointer to a structure, but that
is not what happened.

> However, if you're concerned, I think we could have show_date massage a
> NULL date, like:
>
> diff --git a/date.c b/date.c
> index 8f91569..a04d089 100644
> --- a/date.c
> +++ b/date.c
> @@ -173,6 +173,10 @@ const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
>  {
>  	struct tm *tm;
>  	static struct strbuf timebuf = STRBUF_INIT;
> +	static const struct fallback_mode = { DATE_NORMAL };

Yes, that is nasty.  Renumbering the enum to begin with 1 may be a
much saner solution, unless somebody does

	if (!mode->type)
        	/* we know DATE_NORMAL is zero, he he */
	        do the normal thing;

In any case, I did another evil merge to fix it.