On Mon, Apr 24, 2017 at 12:26 AM, Jan Tulak <jtulak@xxxxxxxxxx> wrote: > On Mon, Apr 24, 2017 at 7:00 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote: >> On Fri, Mar 10, 2017 at 09:34:35AM +1100, Dave Chinner wrote: >>> On Thu, Mar 09, 2017 at 06:57:51PM +0100, Luis R. Rodriguez wrote: >>> > I used reset_opt() and went with "last entry specified wins". From my >>> > review the goal of the respecification was to ensure each opt param >>> > parsed would not reset a prior set param, a paranoid measure, however >>> > this clearly does not work well if we want to allow for "last entry >>> > specified wins", or re-use the validators for a config file parsing >>> > for a first shot a parsing entries. >>> >>> Which is essentially broken, because doing something like: >>> >>> -m crc =1 -m reflink=1 -m crc=0 >>> >>> leaves you with an /invalid config/ because of the respecification >>> of -m crc=0 and the order in which options are parsed and verified. >>> >>> Indeed, things like block and sector sizes are particularly nasty in >>> this respect, because other options can be specified in block or >>> sector units. SO things like: >>> >>> -s size=4k -b size=1s -s size=512 -d size=1000000s >>> >>> were considered valid. respecification of options like this is just >>> borken, and even if we take "last specification wins" it still means >>> that the block size specification is ambiguous and potentially >>> incorrect depending on other options. Hence respecification of >>> options is simply not allowed and post-processing of the options >>> doesn't change that. >> >> We have to pick an approach and stick with, the above seems sensible. >> >>> i.e., the biggest issue with reusing the existing parsing code for >>> the "default config" is that is doesn't just set default values - it >>> prevents other options from being used. >> >> Right as per original design. >> >>> IOWs, the config file should >>> set the default values in the option table, not set the options >>> directly as happens on the command line.... >> >> As I respin my patches addressing concerns an issue I see with this is current >> semantics for "defaultval" is not that they will be the defaults, but rather >> they will be the defaults *iff* the user did specify the option on the command >> line but did not provide an explicit value. This for example would not allow > > Yes, a confusing name. I'm aware of it and have a fix to rename it to .flagval. > It was part of my big set before and now I moved it into the smaller > set I submitted > on this Sunday. The same set adds a new field .value, which can be used to > specify default as in "if the user does not specify this option at all". Terrific, thanks will use that. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
