Re: [PATCH v6 08/12] xfs: refactor suffix_kstrtoint()

Darrick J. Wong
2019-10-16T15:43:12Z

On Wed, Oct 16, 2019 at 01:34:20AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 16, 2019 at 08:41:27AM +0800, Ian Kent wrote:
> > The mount-api doesn't have a "human unit" parse type yet so
> > the options that have values like "10k" etc. still need to
> > be converted by the fs.
> >
> > But the value comes to the fs as a string (not a substring_t
> > type) so there's a need to change the conversion function to
> > take a character string instead.
> >
> > When xfs is switched to use the new mount-api match_kstrtoint()
> > will no longer be used and will be removed.
>
> Please use up the full 72 chars available for the commit log.
>
> > +STATIC int
> > +match_kstrtoint(const substring_t *s, unsigned int base, int *res)
>
> No need for static on new/heavily modified functions, just use static.
>
> Note that both this and suffix_kstrtoint don't really follow the
> normal XFS prototype formatting style either.
>
> > + const char *value;
> > + int ret;
>
> Similarly here - either you follow the XFS style of tab alignining
> the variable names for all variables, or for none, but a mix is very
> odd.

Please follow the xfs style of tab aligning variables inside xfs.

--D