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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2015, 21:05 UTC
Message-ID: <xmqq4mlf3cgf.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150707204837.GA15483@peff.net>

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.

Show 12 quoted lines

> 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.