From: John Keeping <john@keeping.me.uk>
To: Jeff King <peff@peff.net>
Cc: "H.Merijn Brand" <h.m.brand@xs4all.nl>, git@vger.kernel.org
Subject: Re: [PATCH 2/3] convert "enum date_mode" into a struct
Date: Thu, 25 Jun 2015 18:03:28 +0100 [thread overview]
Message-ID: <20150625170328.GV18226@serenity.lan> (raw)
In-Reply-To: <20150625165501.GB23503@peff.net>
On Thu, Jun 25, 2015 at 12:55:02PM -0400, Jeff King wrote:
> In preparation for adding date modes that may carry extra
> information beyond the mode itself, this patch converts the
> date_mode enum into a struct.
>
> Most of the conversion is fairly straightforward; we pass
> the struct as a pointer and dereference the type field where
> necessary. Locations that declare a date_mode can use a "{}"
> constructor. 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.
Yes it does, e.g. in 6.5.2.5 of C11, example 3 shows:
drawline(&(struct point){.x=1, .y=1},
&(struct point){.x=3, .y=4});
The cast is required, but if the argument is pointer-to-const you can
construct a temporary in the function call.
Of course, whether all of the compilers we target support it is a
different question. If they do, perhaps something like:
#define SIMPLE_DATE(f) &(struct date_mode) { DATE_NORMAL }
would allow the callers to remain reasonably sane.
> Likewise, we cannot
> cast the constant to a struct, because we need to pass an
> actual address. Our options are basically:
>
> 1. Manually add a "struct date_mode d = { DATE_NORMAL }"
> definition to each caller, and pass "&d". This makes
> the callers uglier, because they sometimes do not even
> have their own scope (e.g., they are inside a switch
> statement).
>
> 2. Provide a pre-made global "date_normal" struct that can
> be passed by address. We'd also need "date_rfc2822",
> "date_iso8601", and so forth. But at least the ugliness
> is defined in one place.
>
> 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.
next prev parent reply other threads:[~2015-06-25 17:04 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-25 11:19 several date related issues H.Merijn Brand
2015-06-25 12:44 ` Jeff King
2015-06-25 12:56 ` H.Merijn Brand
2015-06-25 16:53 ` [PATCH 0/3] localized date format Jeff King
2015-06-25 16:54 ` [PATCH 1/3] show-branch: use DATE_RELATIVE instead of magic number Jeff King
2015-06-25 16:55 ` [PATCH 2/3] convert "enum date_mode" into a struct Jeff King
2015-06-25 17:03 ` John Keeping [this message]
2015-06-25 17:22 ` Jeff King
2015-07-07 20:37 ` Junio C Hamano
2015-07-07 20:48 ` Jeff King
2015-07-07 21:05 ` Junio C Hamano
2015-07-07 21:13 ` Jeff King
2015-07-07 21:19 ` Junio C Hamano
2015-06-25 16:55 ` [PATCH 3/3] introduce "format" date-mode Jeff King
2015-06-29 22:22 ` Eric Sunshine
2015-06-30 10:20 ` Jeff King
2015-06-30 16:22 ` Junio C Hamano
2015-06-30 17:50 ` Jeff King
2015-06-30 19:23 ` Junio C Hamano
2015-06-30 19:33 ` Jeff King
2015-06-30 16:58 ` Eric Sunshine
2015-06-30 17:58 ` Jeff King
2015-06-30 18:13 ` Eric Sunshine
2015-06-30 19:22 ` Jeff King
2015-06-30 17:05 ` Junio C Hamano
2015-06-30 17:48 ` Eric Sunshine
2015-06-30 19:17 ` Jeff King
2015-06-30 13:26 ` Jeff King
2015-06-30 17:05 ` Eric Sunshine
2015-07-21 0:41 ` Eric Sunshine
2015-07-21 1:19 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150625170328.GV18226@serenity.lan \
--to=john@keeping.me.uk \
--cc=git@vger.kernel.org \
--cc=h.m.brand@xs4all.nl \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.