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
> -Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|