[Toybox] Commit 897: size reduction, and new warning

[Rob Landley]

On 05/14/2013 08:38:08 AM, Isaac Dunham wrote:
> On Mon, 13 May 2013 23:15:48 -0500
> Rob Landley <rob at landley.net> wrote:
> 
> > On 05/12/2013 01:58:12 AM, idunham at lavabit.com wrote:
> > > I'm impressed:
> > > -rwxr-xr-x 1 143640 May 11 23:16 toybox
> > > -rwxr-xr-x 1 252748 May 10 21:32 toybox-895
> > > -rwxr-xr-x 1 166499 May 11 23:16 toybox_unstripped
> > > (Coincidentally, this is the output of ls -lgo with coreutils;
> > > toybox appears to have no way to do a full ls with neither group  
> nor
> > > owner...but I'd expect that one could deal with that via pipes.)
> 
> > > So a change of less than 30 lines nearly halved size.
> >
> > Odd. I'm not seeing that here. Have you tried Have you tried
> > scripts/bloat-o-meter?
> 
> Oops, I think I forgot that I built the first one static with musl and
> the second shared.
> 
> > hg checkout -r 896
> > make allyesconfig
> > make baseline
> > hg checkout -r 897
> > make bloatcheck
> >
> > name                                           old       new
> > delta
> >  
> -----------------------------------------------------------------------
> > xgetmountlist                                   0       243
> > 243 df_main                                       299
> > 294         -5 getmountlist
> > 343         0       -343
> >  
> -----------------------------------------------------------------------
> > -105 total
> 
> Sounds much more reasonable.
> 
> > > But gcc is warning about line 21 of lib/getmountlist.c:
> > >   for (mtlist = 0; me = getmntent(fp); mtlist = mt) {
> >
> > Yeah it wants an extra parentheses to confirm I really MEAN an
> > assignment in the middle there. On the todo list...
> >
> > > Output of make:
> > > scripts/make.sh
> > > Make generated/config.h from .config.
> > > Extract configuration information from toys/*.c files...
> > > Generate headers from toys/*/*.c...
> > > generated/newtoys.h
> > > generated/globals.h
> > > generated/help.h
> > > Extract help text from Config.in.
> > > Library probe...
> > > Compile toybox...
> > > lib/getmountlist.c: In function 'xgetmountlist':
> > > lib/getmountlist.c:21: warning: suggest parentheses around
> > > assignment used as truth value
> > > toys/pending/stat.c: In function 'check_type_file':
> > > toys/pending/stat.c:84: warning: control reaches end of non-void
> > > function
> > >
> > > The latter warning is GCC nonsense, I know.
> >
> > My allyesconfig is also giving warnings from mke2fs and toysh, but
> > both of those need to essentially be restarted...
> 
> This is a custom config including everything I find useful (find xzcat
> ifconfig stat, IIRC-on a different laptop right now, so not certain)  
> as
> well as pretty much all of defconfig.
> toysh is not included for obvious reasons, and I didn't enable  mke2fs
> this time-is it functional?

No, that's what I was working on the first time I mothballed the  
project.

The tricky bit is I was making both mke2fs and gene2fs in the same  
code, and making it work like an archiver so I could point it at a  
bunch of files and have it spit out an ext2 image that you could pipe  
to gzip. This turns out to be hard because you need to do two passes,  
the first precalculating the metadata (so you know where all the block  
allocations go) and the second writing out the actual contents. Doable,  
but really fiddly.

There was also sparse inode allocation stuff that eventually turned  
into ext4, and I got distracted with other things. (Aboriginal wasn't  
near 1.0 yet. The toybox relaunch happend _after_ Aboriginal was at the  
"does what it says on the tin" stage. Yeah there's a whole follow-up  
project making it bootstrap debian and fedora and gentoo and such, but  
I haven't really opened that can of worms yet...)

But I did most of the research and design work. Now I just need to find  
time to implement it...

> > > So gcc seems to be assuming that the condition is
> > > "While assigning the result of getmntent(fp) to me succeeds."
> > > Am I missing something here, or should that have been "=="?
> >
> > No, it's actually an assignment. It reads in the next entry, saving
> > it to the variable, and stops if the entry was null. The test
> > condition is run the first time through the loop and after every
> > increment, so putting it there avoids duplicating it.
> >
> > I should probably have a comment about that...
> 
> Ah, thanks for clarifying.

Yeah, definitely needs a comment. :)

Rob
 1368582282.0