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
