xfs
[Top] [All Lists]

Re: [PATCH 12/15] mkfs: merge getnum

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 12/15] mkfs: merge getnum
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 3 Dec 2013 10:20:28 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131202172233.GD28630@xxxxxxxxxxxxx>
References: <1385689430-10103-1-git-send-email-david@xxxxxxxxxxxxx> <1385689430-10103-13-git-send-email-david@xxxxxxxxxxxxx> <20131202172233.GD28630@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Dec 02, 2013 at 09:22:33AM -0800, Christoph Hellwig wrote:
> On Fri, Nov 29, 2013 at 12:43:47PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > getnum() is now only called by getnum_checked(). Move the two
> > together into a single getnum() function and change all the callers
> > back to getnum().
> 
> So we now have two different getnums in mkfs now.  Maybe the one in
> proto.c should have a different name? 

Probably should.

> 
> > +static long long
> > +getnum(
> > +   const char      *str,
> > +   unsigned int    blksize,
> > +   unsigned int    sectsize,
> > +   bool            convert)
> > +{
> > +   long long       i;
> > +   char            *sp;
> > +
> > +   if (convert)
> > +           return cvtnum(blksize, sectsize, str);
> 
> Also the whole if convert is true sillyness lives on here.  The caller
> that wants cvtnum should just call it directly.

Yes, but soon it doesn't just return the value directly ;)

> > +   else {
> > +           char            *sp;
> > +
> > +           c = strtoll(str, &sp, 0);
> > +           if (c == 0 && sp == str)
> > +                   illegal_option(str, opts, index);
> > +           if (*sp != '\0')
> > +                   illegal_option(str, opts, index);
> > +   }
> 
> And given that the strtoll wrapping code is the same for both getnums
> I suspect we shoud just have a mkfs_strtoll that gets called here,
> and directly by the proto.c callers.

I'll have a look at doing that once everything else falls out.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>