makefs cast daddr_t to off_t before multiplication

[freebsdphab-AX9_cmx.ietfng.org]

Summary

Apparently some large-file systems out there, such as my powerpc64le Linux box, define daddr_t as a 32-bit type, which is sad and stymies cross-building disk images. Cast daddr_t to off_t before doing arithmetic that overflows.

Test Plan

Compile and build disk images using cheribuild

[arichardson]

LGTM

[imp]

What guarantee is there that off_t is going to be better? If we are cross building, wouldn't it be better to ensure we get the right sized data types rather than play whack a mole like this? Daddr_t is a block offset.

usr.sbin/makefs/ffs/buf.h

62 ↗

(On Diff #80264)

Wouldn't it be better to add

#ifdef linux
typedef int64_t freebsd_daddr_t
#define daddr_t freebsd_daddr_t
#endif

towards the top of this file?

[jrtc27]

In D27458#613527, @imp wrote:

What guarantee is there that off_t is going to be better? If we are cross building, wouldn't it be better to ensure we get the right sized data types rather than play whack a mole like this? Daddr_t is a block offset.

It ultimately gets passed to lseek, so if your off_t is 32-bit nothing's going to help you. If we cared about bootstrapping on 32-bit Linux we'd have to enable LFS for lseek to work at which point off_t is then correct again (or explicitly use lseek64 and off64_t).

[jrtc27]

In D27458#613531, @jrtc27 wrote:

In D27458#613527, @imp wrote:

What guarantee is there that off_t is going to be better? If we are cross building, wouldn't it be better to ensure we get the right sized data types rather than play whack a mole like this? Daddr_t is a block offset.

It ultimately gets passed to lseek, so if your off_t is 32-bit nothing's going to help you. If we cared about bootstrapping on 32-bit Linux we'd have to enable LFS for lseek to work at which point off_t is then correct again (or explicitly use lseek64 and off64_t).

We _could_ get away with _just_ including the cast part of the diff and still use daddr_t everywhere else if you want a more minimal diff, but you'd still be limited to BLOCKSIZE * 2^32 bytes on such 64-bit Linux systems.

[imp]

Ok. I'm sold. Forget my suggestion

[jrtc27]

Wes, could you write a better commit message that captures that justification please? Alternatively Alex or I can write one for you when committing if that's fine with you.

[jrtc27]

Hm, on reflection, this doesn't quite do what it claims. Many of the arguments to this function come from fsbtodb and there are lots of daddr_t's floating around. Unless you want to purge all of them from existence (which is notionally wrong as they _are_ block numbers) I'd suggest leaving just the cast and dropping the rest of the diff.

[freebsdphab-AX9_cmx.ietfng.org]

Just the casts now, no function or structure type changes. This appears to pass muster locally.

[jrtc27]

This looks good to me now. Does anyone else have comments?