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

[Kana Steimle]

Other than the commit title saying 'excludes fat16' rather than
'excludes fat32', it looks fine to me, and does everything it's
supposed to do. I don't know if commit titles can be edited, I don't
keep any code on github right now. If it can't easily be changed, I
would just ignore it.

As a side note, firasuke's issue
(https://github.com/landley/toybox/issues/519) can now be closed.


On Fri, Nov 8, 2024 at 10:15 PM Rob Landley <rob at landley.net> wrote:
>
> 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