Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>> Some incompatible command-line options are caught in pick_commits
>> after parse_args has parsed the options and populated the options
>> structure. Change this so that parse_args only parses valid
>> command-line options
[...]
> Alas, I still don't get it. How can I (the fearful reader) demonstrate
> the problem that this fixes? Is it a big problem? How does the patch
> fix it? Does the patch have downsides, or is it mostly risk-free?
To be clearer, here's a quick example in some hypothetical alternate
world:
cherry-pick/revert: do not create a nonsense "struct opts" when given nonsense argv
The --foo and --bar options make no sense together, but if I
run "git cherry-pick --foo --bar" then parse_cherry_pick_option
will return a nonsense struct with both "fooing" and "barring"
set to true, which makes no sense. Currently it's not a
problem since the code that cares is protected by a check:
if (opts.fooing && opts.barring)
die("cannot foo and bar at the same time");
But that is very fragile --- the frob() code-path relies on
opts.fooing and opts.barring not both being true without such
a check and it's only a coincidence that futz() is called
first and makes that check, protecting it.
We can make sure such a nonsensical options struct is never
created by checking right away that --foo and --bar are not
passed together at option-parsing time. Meanwhile, make sure
regressions in maintaining this invariant are caught in the
future by adding an assertion "assert(!opts->fooing ||
!opts->barring)" to both frob() and futz().
The discussion above also applies to --bar and --qux. Fix
that, too.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html