On 2023-10-17 at 00:00:19, Jeff King wrote:
> On Thu, Oct 12, 2023 at 04:09:30PM +0000, brian m. carlson wrote:
>
> > +static unsigned int munge_st_size(off_t st_size) {
> > + unsigned int sd_size = st_size;
> > +
> > + /*
> > + * If the file is an exact multiple of 4 GiB, modify the value so it
> > + * doesn't get marked as racily clean (zero).
> > + */
> > + if (!sd_size && st_size)
> > + return 0x80000000;
> > + else
> > + return sd_size;
> > +}
>
> Coverity complained that the "true" side of this conditional is
> unreachable, since sd_size is assigned from st_size, so the two values
> cannot be both true and false. But obviously we are depending here on
> the truncation of off_t to "unsigned int". I'm not sure if Coverity is
> just dumb, or if it somehow has a different size for off_t.
Technically, on 32-bit machines, you can have a 32-bit off_t if you
don't compile with -D_FILE_OFFSET_BITS=64. However, such a program is
not very useful on modern systems, since it wouldn't be able to handle
files that are 2 GiB or larger, and so everyone considers that a silly
and buggy way to compile programs.
> I don't _think_ this would ever cause confusion in a real compiler, as
> assignment from a larger type to a smaller has well-defined truncation,
> as far as I know.
>
> But I do wonder if an explicit "& 0xFFFFFFFF" would make it more obvious
> what is happening (which would also do the right thing if in some
> hypothetical platform "unsigned int" ended up larger than 32 bits).
Such a hypothetical platform is of course allowed by the C standard, but
it's also going to run near zero real Unix programs or kernels. I am at
this point reflecting on the prudent decision Rust made to make
almost everything an explicit bit size (e.g., u32, i64).
Nonetheless, I'll reroll this with the other things you mentioned, and
I'll switch that "unsigned int" above to "uint32_t", which I think makes
this more obvious.
I don't, however, intend to change the constant from 0x8000000 to 1,
because I think that's a poorer choice, but I'll try to explain it
better in the commit message. (I had originally aimed to avoid editing
it as much as possible since it's not my name on the commit to avoid
Jason getting blamed unnecessarily for any mistakes I might make.)
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
