----- Original Message -----
> From: "Brian Foster" <bfoster@xxxxxxxxxx>
> To: "Dave Chinner" <david@xxxxxxxxxxxxx>
> Cc: "Jan Ťulák" <jtulak@xxxxxxxxxx>, "Dave Chinner" <dchinner@xxxxxxxxxx>, xfs@xxxxxxxxxxx
> Sent: Tuesday, June 30, 2015 1:27:40 PM
> Subject: Re: [PATCH 13/17] mkfs: encode conflicts into parsing table
>
> On Tue, Jun 30, 2015 at 01:57:36PM +1000, Dave Chinner wrote:
> > On Fri, Jun 26, 2015 at 01:17:31PM -0400, Brian Foster wrote:
> > > On Fri, Jun 19, 2015 at 01:02:02PM +0200, Jan Ťulák wrote:
> > > > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > > >
> > > > Many options conflict, so we need to specify which options conflict
> > > > with each other in a generic manner. We already have a "seen"
> > > > variable used for respecification detection, so we can also use this
> > > > code conflict detection. Hence add a "conflicts" array to the sub
> > > > options parameter definition.
> > .....
> > > > @@ -2020,7 +2027,7 @@ _("cannot specify both -m crc=1 and -n
> > > > ftype\n"));
> > > > &value)) {
> > > > case S_LOG:
> > > > case S_SECTLOG:
> > > > - if (ssflag || lssflag)
> > > > + if (lssflag)
> > > > conflict('s', subopts,
> > > > S_SECTSIZE, S_SECTLOG);
> > > > sectorlog = getnum(value, &sopts,
> > > > @@ -2032,7 +2039,7 @@ _("cannot specify both -m crc=1 and -n
> > > > ftype\n"));
> > > > break;
> > > > case S_SIZE:
> > > > case S_SECTSIZE:
> > > > - if (slflag || lslflag)
> > > > + if (lslflag)
> > > > conflict('s', subopts, S_SECTLOG,
> > > > S_SECTSIZE);
> > >
> > > Hmm.. so is the limitation here that we can't do generic conflict
> > > detection across different option structs? If so, I suppose that's not
> > > the end of the world. The cleanup is still well worth it.
> >
> > I just never got around to coding it in a generic fashion - I didn't
> > finish the entire patchset back when I originally wrote it....
> >
>
> Ok. Well I don't know if Jan is up for adding that or what. :) I
> wouldn't be against getting this in as is so it isn't held off longer.
> It still needs a comment though. ;)
>
> > > I wonder if we
> > > still need to set lslflag/lssflag in either of the above cases, though.
> > > It seems like the generic detection should handle it..?
> >
> > In the end it would look at the relevant ->seen flag to determine
> > if there was a cross-option-struct conflict. Essentially, the
> > conflict definition needs to define conflicts via a {group, option}
> > tuple rather than just the {option} it uses now...
> >
>
> Sure. Not a huge deal, but to be clear my comment here was with respect
> to the fact that we set lslflag and lssflag in those two
> S_SECTLOG/S_SECTSIZE blocks. I suspect we still need the flag for the
> L_SECT* conflict, but it looks like the generic code now handles the
> conflict within the 's' group of options. In other words, we have
> duplicate handling of the S_SECTLOG/S_SECTSIZE conflict after this
> patch.
>
> Brian
>
I'm for making it all in the generic code, if you are asking that. :-)
Thanks for pointing out the possible duplicate handling, I will check it. (And thanks for all the others notices, too. :-))
> > -Dave.
> > --
> > Dave Chinner
> > david@xxxxxxxxxxxxx
> >
--
Jan Tulak
jtulak@xxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
