Re: [PATCH] Unbreak makefs -M

On Thu, Sep 11, 2014 at 7:26 PM, Garrett Cooper <yaneurabeya_at_gmail.com>
wrote:

> On Thu, Sep 11, 2014 at 7:16 PM, Davide Italiano <davide_at_freebsd.org>
> wrote:
> > r258695 introduces a sanity check for makefs in order to verify that
> > minimum image size specified is always less than maximum image size.
> >
> > If makefs(1) is invoked specifying minimum image size, but not maximum
> > one, the program exits with an error. Example:
> >
> > # sudo -E makefs -M 538968064 -B be /home/davide/disk.img $DESTDIR
> > makefs: `/home/davide/tftproot/mips' minsize of 538968064 rounded up
> > to ffs bsize of 8192 exceeds maxsize 0.  Lower bsize, or round the
> > minimum and maximum sizes to bsize.
> >
> > I guess it's meaningful to assert that minsize < maxsize iff maxsize
> > is actually specified. The following patch tries to fix the problem.
> > Visual inspection of code also shows that maxsize == 0 is treated as
> > maxsize not specified. I'm not by any means familiar with makefs(1)
> > code, so I may miss something here.
> >
> > % git diff
> > diff --git a/usr.sbin/makefs/ffs.c b/usr.sbin/makefs/ffs.c
> > index 92d5508..83e9eae 100644
> > --- a/usr.sbin/makefs/ffs.c
> > +++ b/usr.sbin/makefs/ffs.c
> > _at__at_ -361,7 +361,8 _at__at_ ffs_validate(const char *dir, fsnode *root,
> > fsinfo_t *fsopts)
> >         if (ffs_opts->avgfpdir == -1)
> >                 ffs_opts->avgfpdir = AFPDIR;
> >
> > -       if (roundup(fsopts->minsize, ffs_opts->bsize) > fsopts->maxsize)
> > +       if (fsopts->maxsize > 0
> > +           && roundup(fsopts->minsize, ffs_opts->bsize) >
> fsopts->maxsize)
>
> - Should roundup be used with fsopts->maxsize) ?
>

No.  Maxsize is a hard limit, while minsize is always rounded up to at
least the bsize.  If you attempt to specify an improperly-rounded quantity
as both the minimum and the maximum (for instance, because the bsize makefs
decides to use has increased between executions), you'll always produce an
image which is too large.  That's exactly what this check was added to
catch (and make less obtuse.)


> - style(9): put the `&&` on the previous line.
>
> >                 errx(1, "`%s' minsize of %lld rounded up to ffs bsize of
> %d "
> >                     "exceeds maxsize %lld.  Lower bsize, or round the
> minimum "
> >                     "and maximum sizes to bsize.", dir,
>
> This (and the other rev) should really be pushed back to NetBSD..
>
> Thanks!
> -Garrett
>