Re: [PATCH] fmt-merge-msg: show those involved in a merged series

On Mon, Mar 12, 2012 at 05:37:57PM -0400, Phil Hord wrote:

Subject: [PATCH] Appease compiler pedantry with an extra cast

Recently git repurposed a pointer as an integer to hold some
counter which git fancies.

Casting directly from 'pointer' to 'int' ((int)(void*)&x) causes a
possible size mismatch because pointers can be bigger than ints.
In such a situation, the compiler complains:

   warning: cast from pointer to integer of different size
            [-Wpointer-to-int-cast]

Yeah, I've been seeing the same warning on my x86_64 box, and came up
with the same fix. However...

Cast the value through intptr_t first to quell compiler complaints
about how this gun appears to be aimed near our feet.  Then cast this
value to an int; this path assures the compiler we are smarter than we
look, or at least that we intend to aim the gun this way for a reason.

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).

I don't think the truncation is a problem in practice, but it just feels
like we are not just silencing an over-zealous compiler, but actually
burying type-size assumption behind a set of four (4!) casts.

I wonder if we would be happier to declare the "util" field of
string_list as a union. Obviously that provides no safety that we read
the correct item out of the union, but that is no worse than the
situation with all of these casts, and I suspect the result would be
much more obvious and readable.

The downside is that all current users of the "util" field would need
s/util/&.ptr/ or similar to dereference the pointer field.

-Peff