[PATCH 03/19] mkfs: Sanitise the superblock feature macros

Jan Tulak jtulak at redhat.com
Thu Apr 7 06:53:39 CDT 2016


On Wed, Apr 6, 2016 at 11:01 PM, Dave Chinner <david at fromorbit.com> wrote:

> On Wed, Apr 06, 2016 at 11:12:21AM +0200, Jan Tulak wrote:
> > On Fri, Apr 1, 2016 at 4:05 AM, Eric Sandeen <sandeen at sandeen.net>
> wrote:
> > > On 3/24/16 6:15 AM, jtulak at redhat.com wrote:
> > >                 if (sb_feat.finobt && sb_feat.finobtflag) {
> > >                         fprintf(stderr,
> > > _("warning: finobt not supported without CRC support, disabled.\n"));
> > >                 }
> > >                 sb_feat.finobt = 0;
> > >
> > > would suffice as before, no? Meh.  Not a big deal I guess....
> > >
> >
> > ​Changed. Honestly, I don't like the strings starting at the beginning of
> > the line, because it breaks the indentation flow, but the rest of the
> code
> > uses this style, so I should stick to it.​
>
> There's good reason for doing this - it makes it easy to grep the
> source code for a specific error that has been emitted. Indentation
> is useful for demonstrating logic flow, but it's harmful when it
> results in strings you might want to find being split up over
> multiple lines.
>

​Mmm, yeah, a valid point. It is caused by the 80 chars limit, but there
are reasons for that too​...


> > Thank you for the review. I will wait a little longer if someone spots
> > something more, before sending an updated patchset.​ :-)
>
> Just send it - I almost got to pulling in this version and
> fixing the various comments directly myself yesterday....
>

​OK. I will fix what Eric submitted in the mean time, test it to be sure I
didn't broke anything and send.

Cheers,
Jan


-- 
Jan Tulak
jtulak at redhat.com / jan at tulak.me
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://oss.sgi.com/pipermail/xfs/attachments/20160407/25dff3c0/attachment-0001.html>


More information about the xfs mailing list