[Toybox] [PATCH] Fix Mac build.

[enh]

On Wed, Mar 11, 2020 at 8:00 PM Rob Landley <rob at landley.net> wrote:
>
> On 3/11/20 9:15 PM, enh wrote:
> > okay, so that was an investigation well worth doing...
> >
> > i tested all the block devices on my mac and linux laptops, and found
> > (a) i was incorrectly letting the linux <<9 apply to darwin's
> > already-in-bytes result ... and then realized that the linux result is
> > already in bytes too (BLKGETSIZE is blocks, but BLKGETSIZE64 is
> > bytes). patch sent.
>
> 64 bits covers a multitude of sins. Sorry I hadn't tested that yet. :(

oh, it gets worse ... when i wrote my test program to compare the
ioctl and lseek, i copy & pasted this code out and corrected the bug
where we were passing the ioctl a pointer to a pointer rather than
just a pointer ... but then i forgot to actually send you that fix.
meanwhile, as i slept others had to diagnose that crash. i've
forwarded you their fix (because i don't know how to get git to keep
them as the author; their original is
https://android-review.googlesource.com/c/platform/external/toybox/+/1255538
if that helps).

> I'm trying to get "make root" set up here so I can actually run the test suite
> in a virtual system under qemu, which would allow me to regression test things
> like "block device size".

yeah, given that you need to be root to even get the size of a block
device (at least on the systems available to me) i don't see how we
can do better with what we currently have. (though obviously getting a
32-bit device into presubmit would help.)

> (The really _fun_ part is you can "truncate -s $(((1<<27)+(1<<18))) blah.img"
> and then feed it into qemu -hdc blah.img and _it_ won't care. The toybox
> infrastructure will still care because a 32 bit build of truncate can't take
> numeric arguments larger than 4G (because it's parsed into a long)...)
>
> Way back when I was thinking "if I do the minimal smb 2.0 posix mod only server,
> I can run it on the host on a high loopback port to export a directory to qemu,
> and then it can loopback mount files out of _that_" which sent me down a "should
> I implement at least the gzip variant of squashfs after doing zip?" and it all
> got pushed into post-1.0 todo. :P

(why smb? wouldn't nfs2 or 9p be orders of magnitude less work?)

> > but with that fixed, i get the same results for the ioctls and the
> > seeks on all my block devices, fwiw.
> >
> > (we could always remove the binary search, see if anyone notices, and
> > only bring it back if they do... has it been 7 years since you last
> > saw a broken kernel?)
>
> ...probably?
>
> It was a broken driver not a broken kernel. And it was "size query IOCTL doesn't
> spin up the disk", so if it hasn't already read/cached the value it either
> returned 0 or failed (I forget) because it didn't even know if a CD or a DVD was
> in the drive.
>
> At the time I went "I don't know what other drivers have this problem", but
> presumably if they do it's a driver bug and the kernel should be fixed.
>
> And even if it's still the case for CD/DVD (which don't really come up much
> anymore), the only thing _conceivably_ inconvenienced by that is ftpget. (Unless
> you're going to call vi or hexedit on /dev/cdrom. Or unless your loadpolicy file
> is a raw CD image.)

amusingly i do have a machine with a CD/DVD drive, but i don't have a
CD or DVD to put in it. (although i'll check later whether a console
game will mount...)

> Possibly this should be xgetfdlength(), where if the ioctl fails it exits
> noisily? (If this is truly now a "this should never happen" situation. It can
> still happen if you haven't got permission to read the file, and in that case
> exiting with the error message is probably the right call for vi and hexedit and
> modinfo and so on...)

if you haven't got permission to read the file, your open() failed and
you either exited then, or passed us -1 :-)

> Rob