On 2023-10-26 06:03, Eric Sunshine wrote:
On Wed, Oct 25, 2023 at 11:52 PM Dragan Simic <dsimic@xxxxxxxxxxx>
wrote:
On 2023-10-26 05:43, Eric Sunshine wrote:
> On Wed, Oct 25, 2023 at 8:55 PM <emilyshaffer@xxxxxxxxxx> wrote:
>> diff --git a/builtin/bugreport.c b/builtin/bugreport.c
>> @@ -126,6 +126,12 @@ int cmd_bugreport(int argc, const char **argv,
>> const char *prefix)
>> + if (argc) {
>> + if (argv[0])
>> + error(_("unknown argument `%s'"), argv[0]);
>> + usage(bugreport_usage[0]);
>> + }
>
> Can it actually happen that argc is non-zero but argv[0] is NULL? (I
> don't have parse-options in front of me to check.) If not, then the
> extra `if (argv[0])` conditional may confuse future readers.
According to https://stackoverflow.com/a/2794171/22330192 it can't,
but
argv[0] can be a zero-length string.
This case is different, though, since, by this point, argv[] has been
processed by Git's parse-options API. Here's the relevant comment from
parse-options.h:
Ah, I see, thanks for the clarification.
* parse_options() will filter out the processed options and leave
the
* non-option arguments in argv[]. argv0 is assumed program name and
* skipped.
*
* Returns the number of arguments left in argv[].
So, I think the `if (argv[0])` conditional is unnecessary, thus
potentially confusing.
It's possible that Emily meant `if (*argv[0])`, but even that seems
undesirable since even a zero-length argv[0] provides some useful
context.
% git bugreport ""
error: unknown argument `'
