[Toybox] [PATCH] Use LOOP_CLR_FD instead of a constant.

[Rich Felker]

On Sun, Mar 29, 2015 at 10:51:04AM -0500, Rob Landley wrote:
> On 03/28/2015 08:20 PM, enh wrote:
> > Use LOOP_CLR_FD instead of a constant.
> > 
> > losetup.c already uses LOOP_CLR_FD.
> > 
> > diff --git a/toys/lsb/umount.c b/toys/lsb/umount.c
> > index c7998e4..d688ad2 100644
> > --- a/toys/lsb/umount.c
> > +++ b/toys/lsb/umount.c
> > @@ -30,6 +30,7 @@ config UMOUNT
> > 
> >  #define FOR_umount
> >  #include "toys.h"
> > +#include <linux/loop.h>
> > 
> >  GLOBALS(
> >    struct arg_list *t;
> > @@ -84,8 +85,7 @@ static void do_umount(char *dir, char *dev, int flags)
> >        int lfd = open(dev, O_RDONLY);
> > 
> >        if (lfd != -1) {
> > -        // This is LOOP_CLR_FD, fetching it from headers is awkward
> > -        if (!ioctl(lfd, 0x4C01) && (toys.optflags & FLAG_v))
> > +        if (!ioctl(lfd, LOOP_CLR_FD) && (toys.optflags & FLAG_v))
> 
> The "awkward" part is that #including a linux/*.h header from a command
> that's been in every unix for over 40 years (whatever posix has to say
> on the matter) seems a bit impolite to bsd and mac.
> 
> I can put some sort of if (LINUX) around the ioctl call if that random
> ioctl turns out to be harmful on those systems (rather than just a
> random -ENOCLUE or similar), but conditionally chopping out a header
> that way is the kind of ugliness I try to hide in portability.h.
> 
> Not a _strong_ reason, but that's why I did it.

I agree with chopping out linux/*.h includes especially since they
tend to randomly break by introducing conflicts with libc headers.
Here it seems your motive is more that you want to avoid making the
command gratuitously Linux-dependent. However I'm not sure if it's
safe to send an 'unknown' ioctl to a random block device you just
opened. Worst case, the number could mean something completely
different like "format the drive". As far as I know Linux only has a
small known number of 'overloaded' ioctl numbers (ones with different
meanings for different device types) and this isn't one of them, but I
don't know whether the number is used for different things on other
systems, so in the absence of that information it might be best to
skip making the ioctl on non-Linux targets.

Rich

 1427646425.0