Jeff King <p...@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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to