On Fri, 2021-11-05 at 15:10 -0500, Benjamin Marzinski wrote: > On Thu, Nov 04, 2021 at 09:11:34PM +0000, Martin Wilck wrote: > > On Wed, 2021-10-06 at 15:04 -0500, Benjamin Marzinski wrote: > > > Add error reporting to the remaining config handlers. If the > > > value is > > > invalid, do not change the existing config option's value. > > > > Like for the previous patch, I'm unsure if this is wise. You rely > > on a > > reasonable default being set before the function is called. I > > suppose > > that's the case, but I like seeing the "invalid" value substituted > > right there where the validity is checked. That saves us from > > searching > > the code for the default value. > > > > Maybe I overlooked an important rationale for not touching the > > values > > in the case of invalid input, please explain. > > Since these handlers are only called if people put the corresponding > option in the config files, we had better have sensible defaults if > they're not called (or if they don't set anything). > > I admit that I should take a look for cases were we cap an out-of- > range > value, to see if it would make more sense to treat it as an invalid > value instead. Also, instead of accepting strings that are simply a > number, we should convert the string, and the check the actual > number. > But I don't see any harm in simply ignoring the invalid values. It's > no > different than if the user didn't put the invalid line into > multipath.conf > > Not setting the values on garbage input makes the handlers more > general. > If you have two options that work the same except that they have > different defaults, then by not explicitly setting the value to the > default when you have invalid input, one handler can be used for both > options. set_yes_no() is a good example. Without my patch, it always > set the value to something, even if the input was garbage. But the > default value it set was "no". That had nothing to do with the > default > value of the options that were using it. You could do extra work to > make > sure that it would correctly use the option's default value, but you > get > the same outcome, with simpler code, just by not changing the default > if > you have a garbage value. > > Also, many of the handlers never set the value on invalid input. I'm > just > making that consistent across all of the handlers. OK, you've convinced me. Thanks Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
