Jeff King [off-list ref] writes:
... However, the tricky case is where we use the
enum labels as constants, like:
show_date(t, tz, DATE_NORMAL);
Ideally we could say:
show_date(t, tz, &{ DATE_NORMAL });
but of course C does not allow that.
...
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.
On Tue, Jul 07, 2015 at 01:37:08PM -0700, Junio C Hamano wrote:
quoted
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