Re: [PATCH] fmt-merge-msg: show those involved in a merged series
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 14, 2012, 03:44 UTC
- Message-ID
- <7vwr6nsv6z.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120313210357.GC27436@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> On Mon, Mar 12, 2012 at 05:37:57PM -0400, Phil Hord wrote: > > This feels so hacky. One of the callsites does: > > elem->util = (void*)((intptr_t)(util_as_int(elem) + 1)); > > which will truncate the value down to an int before replacing it back in > the void pointer. And that truncation is ultimately what the compiler is > warning about, and what we are sneaking around with the extra cast > (because casting between integer sizes of different types is OK, even > though it can cause truncation).
Yeah, I find it utterly disgusting that Phil's compiler is picky about (int)(void *) and insists on (intptr_t)(void *), while totally ignoring the same bit lossage coming from (int)(intptr_t)(void *).
I am again starting to think that the very original was probably the least bad among the yuckies.
As the whole point of the helper macro is to cast the .util field as "some" integral type, I'm tempted to squash this into the v2 patch I posted earlier today. Earlier complaint from René was very sensible that it looked funny to cast xxx_as_int() explicitly to int, but having to cast "some" integral value explicitly to int is required in the context of sprintf() like vararg function, so it would no longer apply to this version.
builtin/fmt-merge-msg.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 4e7196a..1bc6b8b 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -182,7 +182,7 @@ static void add_branch_desc(struct strbuf *out, const char *name) strbuf_release(&desc); } -#define util_as_int(elem) ((int)(intptr_t)((elem)->util)) +#define util_as_integral(elem) ((intptr_t)((elem)->util)) static void record_person(int which, struct string_list *people, struct commit *commit) @@ -210,13 +210,13 @@ static void record_person(int which, struct string_list *people, elem = string_list_insert(people, name_buf); elem->util = (void *)0; } - elem->util = (void*)((intptr_t)(util_as_int(elem) + 1)); + elem->util = (void*)(util_as_integral(elem) + 1); } -static int cmp_string_list_util_as_int(const void *a_, const void *b_) +static int cmp_string_list_util_as_integral(const void *a_, const void *b_) { const struct string_list_item *a = a_, *b = b_; - return util_as_int(b) - util_as_int(a); + return util_as_integral(b) - util_as_integral(a); } static void add_people_count(struct strbuf *out, struct string_list *people) @@ -226,13 +226,13 @@ static void add_people_count(struct strbuf *out, struct string_list *people) else if (people->nr == 2) strbuf_addf(out, "%s (%d) and %s (%d)", people->items[0].string, - util_as_int(&people->items[0]), + (int)util_as_integral(&people->items[0]), people->items[1].string, - util_as_int(&people->items[1])); + (int)util_as_integral(&people->items[1])); else if (people->nr) strbuf_addf(out, "%s (%d) and others", people->items[0].string, - util_as_int(&people->items[0])); + (int)util_as_integral(&people->items[0])); } static void credit_people(struct strbuf *out, @@ -267,11 +267,11 @@ static void add_people_info(struct strbuf *out, if (authors->nr) qsort(authors->items, authors->nr, sizeof(authors->items[0]), - cmp_string_list_util_as_int); + cmp_string_list_util_as_integral); if (committers->nr) qsort(committers->items, committers->nr, sizeof(committers->items[0]), - cmp_string_list_util_as_int); + cmp_string_list_util_as_integral); credit_people(out, authors, 'a'); credit_people(out, committers, 'c');