[Toybox] [PATCH] blkid, mount: fix `blkid -L` and add support for `mount LABEL=...`

[Rob Landley]

On 11/8/24 10:51, Kana Steimle wrote:
> I realized after I sent my last email to you that it didn't go to the
> mailing list because I didn't re-add it to who I'm replying to. Do I
> forward it to the mailing list, or just leave it alone?

We continue on from where we are.

For example, I pushed my local tree to github to unbreak Elliott because 
he pulls on fridays and needed the "QUIET" fix for his llvm toolchain, 
but had your patch applied here without the extra file and pushed that 
up without remembering to revert it first, so I tried to do a quick fix 
(before seeing this) and got it wrong. So I come up with a patch on top 
to undo that.

(I may not be at my best right now. It's been a stressful few days.)

>> No idea what gmail's up to these days.
> I've wanted to switch off of gmail and self-host a mail server instead,
> but the last time I tried that I could only receive emails and not send
> them. I'm just sticking with what works for now.
> 
>> Your new test fails because there's no "fat32.bz2" file. (I'm assuming
>> you have one?)
 >
> I included instructions in the patch on how to create the fat32.bz2 file
> in case you didn't want to trust binary files created by strangers on
> the internet (we all know how that went down with xz),

I did:

   truncate -s 1m fat32.img && mkdosfs -F32 -I -nmyfat32 fat32.img

But the UUID didn't match (and I didn't spot where to specify that in 
the man page), and the host /sbin/blkid was outputting two extra fields 
(LABEL_FATBOOT and BLOCK_SIZE) so I wasn't entirely certain what the 
goal here was...

> but if you don't
> care about that and just want a file you can drop in, I've attached it
> for you.

It's small and not executed, if you can manage an exploit when we do 
nothing but bunzip it (with my code) and run blkid on it (with my code) 
then the problem is my code. :)

>> The if (*type=='v') test is ok for distinguishing between "we just
>> checked for 'vfat' and 'iso9660' so which of the two was it", but in
>> code that's run for every filesystem I'm a lot less comfortable with
>> that.
 >
> I was kind of iffy on the (*type=='v') test as well. Part of me thought I
> should replace it with a (!strcmp(type, "vat")) instead since I was
> moving the line out of the check for the full type names, but I guess
> that slipped my mind. If you think your method of checking !FLAG(U)
> is more efficient though, go for it.

It's roughly equal, but my concern wasn't efficiency, it was potential 
mistaken identity. Don't want to reenact the plot of "the man who knew 
too little" by leaving sharp edges in the codebase.

>> -      if (*type=='v') show_tag("SEC_TYPE", "msdos");
>> +      if (*type=='v' && !FLAG(U)) show_tag("SEC_TYPE", "msdos");
> I added the test `&& fstypes[i].magic_len == 4` in order to differentiate
> between fat12/16 and fat32, since both share the "vfat" type name. It's
> admittedly a bit arbitrary, but iirc it was the first clear difference between
> their structures in fstypes. I would instead do:
> -      if (*type=='v') show_tag("SEC_TYPE", "msdos");
> +      if (*type=='v' && fstypes[i].magic_len == 4 && !FLAG(U))
> show_tag("SEC_TYPE", "msdos");
> I guess that's on me for not leaving a comment to explain why that test
> was there.

Length of the MAGIC matching, not the fs name, the test is to _exclude_ 
fat32, not include it. Also, we're already _inside_ !FLAG(U) the extra 
test is !FLAG(L)

Sigh.

Ok, does commit 2045c952145e look ok to you?

Thanks,

Rob