Re: [PATCH 13/17] mkfs: encode conflicts into parsing table

[Brian Foster <bfoster@redhat.com>]

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@redhat.com>
> > > 
> > > 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@fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs