Re: [PATCH v3 18/24] merge-recursive: consolidate unnecessary fields in merge_options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Elijah Newren <newren@xxxxxxxxx> writes:

> At the end of this series, the "merge-recursive: add sanity checks for
> relevant merge_options" commit adds some assertions that would fail if
> someone passed such a value, regardless of whether this patch was
> included or not.  (Are we worried about people having such a config
> value and should we support it?  It goes against the documented
> values, but I guess someone could have set it anyway even if it seems
> odd to set a value that says, "give me whatever the default is.")

I am somewhat worried about changing the rule on the users without
telling them (and without having a good reason to enforce a tighter
version retroactively).

If we are formally forbidding "[diff] renameLimit = -1" and other
out-of-bound values, (1) assert() will most often turn into noop in
non-debug builds, so the last step of this series may not be such a
good sanity check anyway, (2) "if (condition) BUG();" may be better,
but that's for catching programming errors that made the condition
hold and configuration the end-user had should not trigger it.

So if we want to really catch bogus end-user-supplied values (and we
really do---but the definition of "bogus" may be debatable), the
place to do so is not the "sanity-check at the end", but where we
call get_config_int() and friends.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux