git/parse-options.c, branch v2.50.0

parse-options: introduce precision handling for `OPTION_UNSIGNED`

2025-04-17T15:15:16Z

This commit is the equivalent to the preceding commit, but instead of introducing precision handling for `OPTION_INTEGER` we introduce it for `OPTION_UNSIGNED`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

parse-options: introduce precision handling for `OPTION_INTEGER`

2025-04-17T15:15:15Z

The `OPTION_INTEGER` option type accepts a signed integer. The type of the underlying integer is a simple `int`, which restricts the range of values accepted by such options. But there is a catch: because the caller provides a pointer to the value via the `.value` field, which is a simple void pointer. This has two consequences: - There is no check whether the passed value is sufficiently long to store the entire range of `int`. This can lead to integer wraparound in the best case and out-of-bounds writes in the worst case. - Even when a caller knows that they want to store a value larger than `INT_MAX` they don't have a way to do so. In practice this doesn't tend to be a huge issue because users typically don't end up passing huge values to most commands. But the parsing logic is demonstrably broken, and it is too easy to get the calling convention wrong. Improve the situation by introducing a new `precision` field into the structure. This field gets assigned automatically by `OPT_INTEGER_F()` and tracks the size of the passed value. Like this it becomes possible for the caller to pass arbitrarily-sized integers and the underlying logic knows to handle it correctly by doing range checks. Furthermore, convert the code to use `strtoimax()` intstead of `strtol()` so that we can also parse values larger than `LONG_MAX`. Note that we do not yet assert signedness of the passed variable, which is another source of bugs. This will be handled in a subsequent commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

parse-options: rename `OPT_MAGNITUDE()` to `OPT_UNSIGNED()`

2025-04-17T15:15:15Z

With the preceding commit, `OPT_INTEGER()` has learned to support unit factors. Consequently, the major differencen between `OPT_INTEGER()` and `OPT_MAGNITUDE()` isn't the support of unit factors anymore, as both of them do support them now. Instead, the difference is that one handles signed and the other handles unsigned integers. Adapt the name of `OPT_MAGNITUDE()` accordingly by renaming it to `OPT_UNSIGNED()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

parse-options: support unit factors in `OPT_INTEGER()`

2025-04-17T15:15:15Z

There are two main differences between `OPT_INTEGER()` and `OPT_MAGNITUDE()`: - The former parses signed integers whereas the latter parses unsigned integers. - The latter parses unit factors like 'k', 'm' or 'g'. While the first difference makes obvious sense, there isn't really a good reason why signed integers shouldn't support unit factors, too. This inconsistency will also become a bit of a problem with subsequent commits, where we will fix a couple of callsites that pass an unsigned integer to `OPT_INTEGER()`. There are three options: - We could adapt those users to instead pass a signed integer, but this would needlessly extend the range of accepted integer values. - We could convert them to use `OPT_MAGNITUDE()`, as it only accepts unsigned integers. But now we have the inconsistency that we also start to accept unit factors. - We could introduce `OPT_UNSIGNED()` as equivalent to `OPT_INTEGER()` so that it knows to only accept unsigned integers without unit suffix. Introducing a whole new option type feels a bit excessive. There also isn't really a good reason why `OPT_INTEGER()` cannot be extended to also accept unit factors: all valid values passed to such options cannot have a unit factors right now, so there wouldn't be any ambiguity. Refactor `OPT_INTEGER()` to use `git_parse_int()`, which knows to interpret unit factors. This removes the inconsistency between the signed and unsigned options so that we can easily fix up callsites that pass the wrong integer type right now. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'jc/show-usage-help'

2025-01-28T21:02:22Z

The help text from "git $cmd -h" appear on the standard output for some $cmd and the standard error for others. The built-in commands have been fixed to show them on the standard output consistently. * jc/show-usage-help: builtin: send usage() help text to standard output oddballs: send usage() help text to standard output builtins: send usage_with_options() help text to standard output usage: add show_usage_if_asked() parse-options: add show_usage_with_options_if_asked() t0012: optionally check that "-h" output goes to stdout

parse-options: add show_usage_with_options_if_asked()

2025-01-17T21:30:02Z

Many commands call usage_with_options() when they are asked to give the help message, but it sends the help text to the standard error stream. When the user asked for it with "git cmd -h", the help message is the primary output from the command, hence we should send it to the standard output stream, instead. Introduce a helper function that captures the common pattern if (argc == 2 && !strcmp(argv[1], "-h")) usage_with_options(usage, options); and replaces it with show_usage_with_options_if_asked(argc, argv, usage, options); to help correct code paths. Note that this helper function still exits with status 129, and t0012 insists on it. After converting all the mistaken callers of usage_with_options() to call this new helper, we may want to address it---the end user is asking us to give the help text, and we are doing exactly as asked, so there is no reason to exit with non-zero status. Suggested-by: Jeff King Signed-off-by: Junio C Hamano

parse-options: localize mark-up of placeholder text in the short help

2024-12-30T14:55:24Z

i18n: expose substitution hint chars in functions and macros to translators For example (based on builtin/commit.c and shortened): the "--author" option takes a name. In source this can be represented as: OPT_STRING(0, "author", &force_author, N_("author"), N_("override author")), When the command is run with "-h" (short help) option (git commit -h), the above definition is displayed as: --[no-]author override author Git does not use translated option names so the first part of the above, "--[no-]author", is given as-is (it is based on the 2nd argument of OPT_STRING). However the string "author" in the pair of "<>", and the explanation "override author for commit" may be translated into user's language. The user's language may use a convention to mark a replaceable part of the command line (called a "placeholder string") differently from enclosing it inside a pair of "<>", but the implementation in parse-options.c hardcodes "<%s>". Allow translators to specify the presentation of a placeholder string for their languages by overriding the "<%s>". In case the translator's writing system is sufficiently different than Latin the "<>" characters can be substituted by an empty string thus effectively skipping them in the output. For example languages with uppercase versions of characters can use that to deliniate replaceability. Alternatively a translator can decide to use characters that are visually close to "<>" but are not interpreted by the shell. Signed-off-by: Alexander Shopov Signed-off-by: Junio C Hamano

parse-options: free previous value of `OPTION_FILENAME`

2024-09-27T15:25:35Z

The `OPTION_FILENAME` option always assigns either an allocated string or `NULL` to the value. In case it is passed multiple times it does not know to free the previous value though, which causes a memory leak. Refactor the function to always free the previous value. None of the sites where this option is used pass a string constant, so this change is safe. While at it, fix the argument of `fix_filename()` to be a string constant. The only reason why it's not is because we use it as an in-out-parameter, where the input is a constant and the output is not. This is weird and unnecessary, as we can just return the result instead of using the parameter for this. This leak is being hit in t7621, but plugging it alone does not make the test suite pass. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

parse-options: rearrange long_name matching code

2024-03-03T17:49:22Z

Move the code for handling a full match of long_name first and get rid of negations. Reduce the indent of the code for matching abbreviations and remove unnecessary curly braces. Combine the checks for whether negation is allowed and whether arg is "n", "no" or "no-" because they belong together and avoid a continue statement. The result is shorter, more readable code. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

parse-options: normalize arg and long_name before comparison

2024-03-03T17:49:22Z

Strip "no-" from arg and long_name before comparing them. This way we no longer have to repeat the comparison with an offset of 3 for negated arguments. Note that we must not modify the "flags" value, which tracks whether arg is negated, inside the loop. When registering "--n", "--no" or "--no-" as abbreviation for any negative option, we used to OR it with OPT_UNSET and end the loop. We can simply hard-code OPT_UNSET and leave flags unchanged instead. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano