git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Lars Kellogg-Stedman <lars@oddbit.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3] line-range: Fix infinite loop bug with degenerate '$' regex
Date: Wed, 14 Dec 2022 09:53:41 -0500	[thread overview]
Message-ID: <20221214145341.sonppjtshwqoxs6n@oddbit.com> (raw)
In-Reply-To: <xmqq5yeiwr6x.fsf@gitster.g>

On Sun, Dec 11, 2022 at 12:32:38PM +0900, Junio C Hamano wrote:
> Lars Kellogg-Stedman <lars@oddbit.com> writes:
> 
> > When the -L argument to "git log" is passed the degenerate regular
> > expression "$" (as in "-L :$:line-range.c"), this results in an
> > infinite loop in find_funcname_matching_regexp().
> 
> Is "matching an empty line" the only way a regular expression can be
> a "degenerate" one?  If not, perhaps being a bit more explicit would
> help the readers, e.g.

"Matching an empty line" isn't the issue (and if I implied that it
was, that's my bad). The issue only crops up when matching the
zero-width regular expression at the *end* of a line.

I'll update the subject and description to be more clear...

> This clear explanation probably deserves to be in the commit log
> message proper.

...including adding the extended description to the commit message. Some projects
object if commit messages get too wordy.

> Please write it on two lines, and highlight an empty body of the
> loop, like so

Will do.

> > @@ -161,6 +160,7 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
> >  			return bol;
> >  		start = eol;
> >  	}
> > +	return NULL;
> >  }
> 
> What is this change about?  Isn't the above an endless loop without
> break, from which the only way for the control to leave it is by
> returning?

It's not an endless loop without break; this change modified the loop
condition:

> -       while (1) {
> +       while (*start) {

That would have prevented the infinite loop in the first place. I'm
happy to drop this change if you prefer, but if this condition had
been in place originally we wouldn't have had the infinite loop bug
(we would still have had incorrect behavior, but it would have been
perhaps easier for an end user to diagnose).

> Style: lose the SP after "cat >".

Will do.

-- 
Lars Kellogg-Stedman <lars@oddbit.com> | larsks @ {irc,twitter,github}
http://blog.oddbit.com/                | N1LKS

  parent reply	other threads:[~2022-12-14 14:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-05 19:36 [PATCH v2] line-range: Fix infinite loop bug with degenerate regex Lars Kellogg-Stedman
2022-12-07  4:33 ` Eric Sunshine
2022-12-07  4:52 ` Ævar Arnfjörð Bjarmason
2022-12-07  5:29   ` Junio C Hamano
2022-12-07 20:30   ` SZEDER Gábor
2022-12-09 19:16   ` Lars Kellogg-Stedman
2022-12-11  1:53 ` [PATCH v3] line-range: Fix infinite loop bug with degenerate '$' regex Lars Kellogg-Stedman
2022-12-11  3:32   ` Junio C Hamano
2022-12-11  3:34     ` Junio C Hamano
2022-12-14 14:53     ` Lars Kellogg-Stedman [this message]
2022-12-18  1:33       ` Junio C Hamano
2022-12-19 22:48 ` [PATCH v4] line-range: fix infinite loop bug with " Lars Kellogg-Stedman
2022-12-19 22:55   ` Ævar Arnfjörð Bjarmason
2022-12-20  0:00     ` Eric Sunshine
2022-12-20  1:07       ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221214145341.sonppjtshwqoxs6n@oddbit.com \
    --to=lars@oddbit.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).