Hi Geert,
Am 16.06.2023 um 19:28 schrieb Geert Uytterhoeven:
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.
Thanks, that explains why the result of checksum_block() is tested
against zero. Makes sense now.
Will fix in v12 ...
Cheers,
Michael
Gr{oetje,eeting}s,
Geert
