Re: [PATCH v10 2/3] block: change annotation of rdb_CylBlocks in affs_hardblocks.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

On Fri, Jun 16, 2023 at 9:21 AM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
Am 16.06.2023 um 17:48 schrieb Christoph Hellwig:
On Fri, Jun 16, 2023 at 07:53:11AM +1200, Michael Schmitz wrote:
Thanks - now there's two __s32 fields in that header - one checksum each
for RDB and PB. No one has so far seen the need for a 'signed big endian 32
bit' type, and I'd rather avoid adding one to types.h. I'll leave those as
they are (with the tacit understanding that they are equally meant to be
big endian).

We have those in a few other pleases and store them as __be32 as well.  The
(implicit) cast to s32 will make them signed again.

Where's that cast to s32 hidden? I've only seen

#define __be32_to_cpu(x) ((__force __u32)(__be32)(x))

which would make the checksums unsigned if __be32 was used.

Whether the checksum code uses signed or unsigned math would require
inspection of the Amiga partitioning tool source which I don't have, so
I've kept __s32 to be safe.

Unsurprisingly, block/partitions/amiga.c:checksum_block() calculates
a checksum over __be32 words.  The actual signedness of the checksum
field doesn't matter much[*], as using two-complement numbers, you can
just assign a signed value to an unsigned field.
It should definitely be __be32.

[*] I guess it was made signed because the procedure to update the
check goes like this:
  1. set checksum field to zero,
  2. calculate sum,
  3. store negated sum in checksum field.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux