On Tue, Mar 13, 2012 at 5:03 PM, Jeff King <peff@xxxxxxxx> wrote: > 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. Well, that's because it is hacky. But it's the original code that's suspect. > 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 think this one is ok because it's really just "the hacky" bit storing an integer in a variable meant to hold a pointer. That's why it's incrementing here, I suppose, not because it really wants to point at the next byte. > 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. The compiler is doing its job here to warn us against storing big-ish pointers in small-ish ints. But if we know we will never accidentally use this as a pointer and if the integer will never overflow the 32-bounds of the (int) representation, then it's all good. But I agree, it is hacky. Phil -- 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