Jeff King <peff@xxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html