[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [BusyBox] [PATCH] Make mount smaller.
From:       Rob Landley <rob () landley ! net>
Date:       2004-10-26 22:33:38
Message-ID: 200410261733.38193.rob () landley ! net
[Download RAW message or body]

On Tuesday 26 October 2004 06:59, Vladimir N. Oleynik wrote:
> Rob,
>
> > --- util-linux/mount.bak	2004-10-26 02:28:36.000000000 -0500
> > +++ util-linux/mount.c	2004-10-26 05:52:34.276929080 -0500
> > @@ -250,72 +250,40 @@
> >  					 int mount_all)
> >  {
> >  	int status = 0;
> > +	/* For "auto", loop through all filesystems listed in /etc and /proc */
> >  	if (strcmp(filesystemType, "auto") == 0) {
> > -		char buf[255];
> > +		char *filesystems[]={"/etc/filesystems","/proc/filesystems",NULL},
> > +			 *fs,*line,*temp;
>
> May be const char * const filesystems[] better.
>
> > +				/* don't try to mount nodev filesystems */
> > +				if (strncmp(line,"nodev\t",6)) {
>
> But /etc/filesystems may be not have "\t" but have " "
> I think, we can move this check to ***

/etc/filesystems shouldn't have "nodev" entries in it.  If the old code saw a 
nodev entry, it would null terminate at the first whitespace after it 
(ignoring it) and try to mount -t nodev (which wouldn't work).

> > +					filesystemType = line;
> > +
> > +					/* Skip leading whitespace, trim trailing whitespace */
> > +					while (isspace(*filesystemType)) filesystemType++;
> > +					for (temp = filesystemType; *temp && !isspace(*temp); temp++);
> > +					*temp = '\0';
> >
> > -					if (bb_strlen(filesystemType)) {
> > +					if (*filesystemType && *filesystemType!='#'
> > +						&& *filesystemType!='*') {
>
> *** as strcmp(), not as strncmp().

All I care is that the first character is an *, which can't be part of a valid 
filename.  I'm treating it as a comment.  A filesystem named "*" is a special 
entry in /etc/filesystems to say "even though /etc/filesystems exists, read 
/proc/filesystems anyway".  A better behavior seems to be just read both and 
use the data you find.  (Is there ever a case where /etc/filesystems is put 
in to _disable_ some legitimate filesystems that are in /proc/filesystems?  I 
can see it being there if we have no /proc...)

I could use "*" to set a flag and only fall through if it's set, but that 
makes the code bigger and more complicated without making it more useful, as 
far as I can see.

It sounds like the _logical_ behavior is actually read /proc/fiilesystems 
first and fall back to /etc/filesystems if we can't open it (or none of the 
filesystems mounted).  Right now there's one trick you can pull with the old 
code that the new code doesn't allow (using /etc/filesystems without an "*" 
entry to hide filesystems from mount -a), but I can't think of a circumstance 
where this trick would actually be useful.

And what you'd probably _want_ to do is put the appropriate filesystem in 
/etc/fstab and teach busybox mount about that...

> --w
> vodz

Rob

_______________________________________________
busybox mailing list
busybox@mail.busybox.net
http://codepoet.org/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]