On Fri, Jan 17, 2025 at 05:27:21PM -0800, Junio C Hamano wrote:
> static inline void put_be32(void *ptr, uint32_t value)
> {
> unsigned char *p = ptr;
> p[0] = value >> 24;
> p[1] = value >> 16;
> p[2] = value >> 8;
> p[3] = value >> 0;
> }
>
> But sparse seems not to like that.
>
> compat/bswap.h:175:22: error: cast truncates bits from constant value (5041 becomes 41)
> compat/bswap.h:176:22: error: cast truncates bits from constant value (504143 becomes 43)
> compat/bswap.h:177:22: error: cast truncates bits from constant value (5041434b becomes 4b)
>
> Of course we could do the mask, but should we have to?
Cute. I think the above is well defined in terms of the C standard. But
I could see how a linter might want to remind you that you're truncating
a constant.
It is kind of lame that it only flags the call with a constant. If you
want to warn people that they are accidentally truncating, surely it's
obvious in the code above that truncation is _possible_ depending on the
value. It seems like it's either worth flagging as a dangerous
construct, or not; but doing it only for a constant is not super
helpful.
> I think the real compiler would be clever ehough to produce the
> identical binary with the following patch that is only needed to
> squelch this error, but I feel dirty after writing this.
I checked with "gcc -s" and it produces the same asm before and after
your patch, with both -O0 and -O2. So I don't think there's a practical
downside. As far as feeling dirty, I dunno. It is basically telling any
linter "yes, I know we are truncating here". Since it is contained
within put_be32() and won't spread across the code base, I'm not too
offended by it.
I guess the other option is to pass -Wno-cast-truncate to sparse.
> By the way, a "git grep" finds
>
> put_be32(&hdr_version, INDEX_EXTENSION_VERSION2);
>
> in the fsmonitor.c file, which does not get flagged only because the
> CPP macro expands to a small integer (2). That is doubly insulting.
Heh. Yeah, that goes back to my "kind of lame" comment above. ;)
-Peff
