From: Junio C Hamano <gitster@pobox.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: git@vger.kernel.org, "Arnfjörð Bjarmason" <avarab@gmail.com>,
"John Cai" <johncai86@gmail.com>
Subject: Re: [PATCH] reflog: fix 'show' subcommand's argv
Date: Mon, 28 Mar 2022 15:45:02 -0700 [thread overview]
Message-ID: <xmqqa6d9zogh.fsf@gitster.g> (raw)
In-Reply-To: <20220328212152.589491-1-szeder.dev@gmail.com> ("SZEDER Gábor"'s message of "Mon, 28 Mar 2022 23:21:52 +0200")
SZEDER Gábor <szeder.dev@gmail.com> writes:
> cmd_reflog() invokes parse_options() with PARSE_OPT_KEEP_ARGV0, but it
> doesn't account for the retained argv[0] before invoking
> cmd_reflog_show() to handle the 'git reflog show' subcommand.
> Consequently, cmd_reflog_show() always gets an 'argv' array starting
> with elements argv[0]="reflog" and argv[1]="show".
>
> Strip the name of the git command from the 'argv' array before passing
> it to the function handling the 'show' subcommand.
>
> There is no user-visible bug here, because cmd_reflog_show() doesn't
> have any options or parameters of its own.
These two changes cancel out as far as cmd_log_reflog() is concerned
but makes a difference inside cmd_reflog_show(), if that function
cared.
There is parse_options() call that uses argc and argv supplied by
the caller, and it is not adjusted, so it is curious why there is no
externally observable behaviour change. But argv[0] and argv[1] are
not dashed options and because we do not say STOP_AT_NON_OPTION,
parse_options() will skip such an extra non-option argument, so is
that the reason why there is no behaviour change?
As this is an "oops, that is not quite right" fix for
ab/reflog-parse-options topic, let's queue it diretly on top of the
topic.
Thanks.
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
> builtin/reflog.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 6c4fe1af40..c943c2aabe 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -225,7 +225,7 @@ static int cmd_reflog_show(int argc, const char **argv, const char *prefix)
> PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0 |
> PARSE_OPT_KEEP_UNKNOWN);
>
> - return cmd_log_reflog(argc - 1, argv + 1, prefix);
> + return cmd_log_reflog(argc, argv, prefix);
> }
>
> static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
> @@ -425,7 +425,7 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)
> goto log_reflog;
>
> if (!strcmp(argv[1], "show"))
> - return cmd_reflog_show(argc, argv, prefix);
> + return cmd_reflog_show(argc - 1, argv + 1, prefix);
> else if (!strcmp(argv[1], "expire"))
> return cmd_reflog_expire(argc - 1, argv + 1, prefix);
> else if (!strcmp(argv[1], "delete"))
prev parent reply other threads:[~2022-03-28 22:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-17 18:08 [PATCH 0/8] reflog: migrate fully to parse_options() Ævar Arnfjörð Bjarmason
2022-03-17 18:08 ` [PATCH 1/8] reflog.c: indent argument lists Ævar Arnfjörð Bjarmason
2022-03-17 19:42 ` John Cai
2022-03-17 18:08 ` [PATCH 2/8] reflog: refactor cmd_reflog() to "if" branches Ævar Arnfjörð Bjarmason
2022-03-17 18:08 ` [PATCH 3/8] reflog tests: add missing "git reflog exists" tests Ævar Arnfjörð Bjarmason
2022-03-17 18:08 ` [PATCH 4/8] reflog: move "usage" variables and use macros Ævar Arnfjörð Bjarmason
2022-03-17 18:08 ` [PATCH 5/8] git reflog [expire|delete]: make -h output consistent with SYNOPSIS Ævar Arnfjörð Bjarmason
2022-03-18 1:24 ` Junio C Hamano
2022-03-17 18:08 ` [PATCH 6/8] reflog exists: use parse_options() API Ævar Arnfjörð Bjarmason
2022-03-18 1:30 ` Junio C Hamano
2022-03-17 18:08 ` [PATCH 7/8] reflog: convert to " Ævar Arnfjörð Bjarmason
2022-03-18 1:49 ` Junio C Hamano
2022-03-17 18:08 ` [PATCH 8/8] reflog [show]: display sensible -h output Ævar Arnfjörð Bjarmason
2022-03-28 21:21 ` [PATCH] reflog: fix 'show' subcommand's argv SZEDER Gábor
2022-03-28 22:45 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa6d9zogh.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=johncai86@gmail.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.