From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>,
git@vger.kernel.org,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH 2/3] range-diff: handle commit ranges other than A..B
Date: Fri, 22 Jan 2021 17:20:08 +0100 (CET) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.2101221713230.52@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <xmqq1redc0x9.fsf@gitster.c.googlers.com>
Hi Junio,
On Thu, 21 Jan 2021, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
> > described to specify commit ranges that `range-diff` does not yet
> > accept: "<commit>^!" and "<commit>^-<n>".
> >
> > Let's accept them.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/range-diff.c | 21 ++++++++++++++++++++-
> > t/t3206-range-diff.sh | 8 ++++++++
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/range-diff.c b/builtin/range-diff.c
> > index 551d3e689cb..6097635c432 100644
> > --- a/builtin/range-diff.c
> > +++ b/builtin/range-diff.c
> > @@ -13,7 +13,26 @@ NULL
> >
> > static int is_range(const char *range)
> > {
> > - return !!strstr(range, "..");
> > + size_t i;
> > + char c;
> > +
> > + if (strstr(range, ".."))
> > + return 1;
> > +
> > + i = strlen(range);
> > + c = i ? range[--i] : 0;
> > + if (c == '!')
> > + i--; /* might be ...^! or ...^@ */
>
> I am confused. If it ends with '!', I do not see how it can end
> with "^@".
Bah. This is a left-over from an earlier version. I tried to hide the fact
that I had misunderstood `<rev>^@` to specify a commit range. Oh well.
> If the input were "!", i gets strlen("!") which is 1, c gets '!'
> while predecrementing i down to 0, and we notice c is '!' and
> decrement i again to make it (size_t)(-1) which is a fairly large
> number.
Right, guarding that `range[--i]` only by `i` is not enough. My idea was
to exit early if the string is too short, anyway, i.e. if `i < 3`.
> Then we skip all the else/if cascade, ensure that i is positive, and
> happily access range[i], which likely is way out of bounds (but it
> probably is almost one turn around the earth out of bounds, it may
> access just a single byte before the array).
>
> Am I reading the code right?
>
> IOW, "git range-diff \! A..B" would do something strange, I would
> guess.
Right. Will be fixed in the next iteration.
Thanks,
Dscho
next prev parent reply other threads:[~2021-01-22 16:22 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 22:20 [PATCH 0/3] Range diff with ranges lacking dotdot Johannes Schindelin via GitGitGadget
2021-01-21 22:20 ` [PATCH 1/3] range-diff: refactor check for commit range Johannes Schindelin via GitGitGadget
2021-01-21 23:27 ` Junio C Hamano
2021-01-22 19:12 ` Phillip Wood
2021-01-22 21:59 ` Junio C Hamano
2021-01-23 15:59 ` Phillip Wood
2021-01-26 15:19 ` Johannes Schindelin
2021-01-21 22:20 ` [PATCH 2/3] range-diff: handle commit ranges other than A..B Johannes Schindelin via GitGitGadget
2021-01-21 23:37 ` Eric Sunshine
2021-01-22 16:12 ` Johannes Schindelin
2021-01-21 23:42 ` Junio C Hamano
2021-01-22 16:20 ` Johannes Schindelin [this message]
2021-01-21 22:20 ` [PATCH 3/3] range-diff(docs): explain how to specify commit ranges Johannes Schindelin via GitGitGadget
2021-01-21 23:46 ` Junio C Hamano
2021-01-22 16:21 ` Johannes Schindelin
2021-01-22 18:21 ` Junio C Hamano
2021-01-27 3:01 ` Johannes Schindelin
2021-01-28 5:43 ` Junio C Hamano
2021-01-22 18:20 ` Uwe Kleine-König
2021-01-26 15:22 ` Johannes Schindelin
2021-01-22 7:31 ` [PATCH 0/3] Range diff with ranges lacking dotdot Uwe Kleine-König
2021-01-22 18:16 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2021-01-22 18:16 ` [PATCH v2 1/3] range-diff/format-patch: refactor check for commit range Johannes Schindelin via GitGitGadget
2021-01-22 20:27 ` Junio C Hamano
2021-01-25 7:35 ` Uwe Kleine-König
2021-01-25 19:24 ` Junio C Hamano
2021-01-25 21:25 ` Uwe Kleine-König
2021-01-26 19:25 ` Junio C Hamano
2021-01-22 18:16 ` [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B Johannes Schindelin via GitGitGadget
2021-01-22 20:32 ` Junio C Hamano
2021-01-27 2:57 ` Johannes Schindelin
2021-01-28 5:57 ` Junio C Hamano
2021-01-28 15:38 ` Johannes Schindelin
[not found] ` <xmqqim7gx41d.fsf@gitster.c.googlers.com>
2021-02-06 0:39 ` Johannes Schindelin
2021-01-22 18:16 ` [PATCH v2 3/3] range-diff(docs): explain how to specify commit ranges Johannes Schindelin via GitGitGadget
2021-01-27 16:37 ` [PATCH v3 0/3] Range diff with ranges lacking dotdot Johannes Schindelin via GitGitGadget
2021-01-27 16:37 ` [PATCH v3 1/3] range-diff/format-patch: refactor check for commit range Johannes Schindelin via GitGitGadget
2021-01-27 16:37 ` [PATCH v3 2/3] range-diff/format-patch: handle commit ranges other than A..B Johannes Schindelin via GitGitGadget
2021-01-27 16:37 ` [PATCH v3 3/3] range-diff(docs): explain how to specify commit ranges Johannes Schindelin via GitGitGadget
2021-02-04 9:31 ` [PATCH v4 0/3] Range diff with ranges lacking dotdot Johannes Schindelin via GitGitGadget
2021-02-04 9:31 ` [PATCH v4 1/3] range-diff/format-patch: refactor check for commit range Johannes Schindelin via GitGitGadget
2021-02-04 18:56 ` Junio C Hamano
2021-02-04 19:27 ` Johannes Schindelin
2021-02-04 9:31 ` [PATCH v4 2/3] range-diff/format-patch: handle commit ranges other than A..B Johannes Schindelin via GitGitGadget
2021-02-04 18:51 ` Junio C Hamano
2021-02-04 21:42 ` Johannes Schindelin
2021-02-04 18:58 ` Junio C Hamano
2021-02-04 21:57 ` Johannes Schindelin
2021-02-04 9:31 ` [PATCH v4 3/3] range-diff(docs): explain how to specify commit ranges Johannes Schindelin via GitGitGadget
2021-02-04 18:53 ` Junio C Hamano
2021-02-04 21:58 ` Johannes Schindelin
2021-02-04 22:42 ` Junio C Hamano
2021-02-04 23:29 ` [PATCH v5 0/3] Range diff with ranges lacking dotdot Johannes Schindelin via GitGitGadget
2021-02-04 23:29 ` [PATCH v5 1/3] range-diff/format-patch: refactor check for commit range Johannes Schindelin via GitGitGadget
2021-02-04 23:29 ` [PATCH v5 2/3] range-diff/format-patch: handle commit ranges other than A..B Johannes Schindelin via GitGitGadget
2021-02-05 1:07 ` Junio C Hamano
2021-02-05 1:32 ` Junio C Hamano
2021-02-05 14:09 ` Johannes Schindelin
2021-02-04 23:29 ` [PATCH v5 3/3] range-diff(docs): explain how to specify commit ranges Johannes Schindelin via GitGitGadget
2021-02-05 14:44 ` [PATCH v6 0/3] Range diff with ranges lacking dotdot Johannes Schindelin via GitGitGadget
2021-02-05 14:44 ` [PATCH v6 1/3] range-diff/format-patch: refactor check for commit range Johannes Schindelin via GitGitGadget
2021-02-05 14:44 ` [PATCH v6 2/3] range-diff/format-patch: handle commit ranges other than A..B Johannes Schindelin via GitGitGadget
2021-02-05 14:44 ` [PATCH v6 3/3] range-diff(docs): explain how to specify commit ranges Johannes Schindelin via GitGitGadget
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=nycvar.QRO.7.76.6.2101221713230.52@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=u.kleine-koenig@pengutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).