On Tue, Jul 07, 2015 at 01:37:08PM -0700, Junio C Hamano wrote:
> > 3. Provide a wrapper that generates the correct struct on
> > the fly. The big downside is that we end up pointing to
> > a single global, which makes our wrapper non-reentrant.
> > But show_date is already not reentrant, so it does not
> > matter.
> >
> > This patch implements 3, along with a minor macro to keep
> > the size of the callers sane.
>
> Another big downside is that DATE_NORMAL is defined to be "0".
>
> This makes it very cumbersome to merge a side branch that uses an
> outdated definition of show_date() and its friends and tell them
> to show date normally. The compiler does not help detecting
> places that need to be adjusted during merge and instead just pass
> a NULL pointer as a pointer to the new struct.
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).
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 };
+
+ if (!mode)
+ mode = &fallback_mode;
if (mode->type == DATE_RAW) {
strbuf_reset(&timebuf);
That would also allow people to explicitly call:
show_date(t, tz, NULL);
to get the default format, though I personally prefer spelling it out.
I guess we _could_ introduce:
#define DATE_MODE(x) ((struct date_mode *)(x))
and then take any numeric value, under the assumption that the first
page of memory will never be a valid pointer:
diff --git a/date.c b/date.c
index 8f91569..f388fee 100644
--- a/date.c
+++ b/date.c
@@ -173,6 +173,18 @@ const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
{
struct tm *tm;
static struct strbuf timebuf = STRBUF_INIT;
+ struct date_mode fallback;
+
+ /* hysterical compatibility */
+ if (mode < 1024) {
+ if (mode == DATE_STRFTIME)
+ die("BUG: nice try");
+ fallback.type = mode;
+ mode = &fallback;
+ }
+
+ if (!mode)
+ mode = &fallback_mode;
if (mode->type == DATE_RAW) {
strbuf_reset(&timebuf);
That's kind of nasty, but at least it's hidden from the callers.
-Peff
--
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