[PATCH 06 of 10 V2] revlog: add a native implementation of issnapshot

Yuya Nishihara yuya at tcha.org
Fri Dec 28 00:21:14 EST 2018


On Fri, 28 Dec 2018 00:26:54 +0100, Boris FELD wrote:
> On 22/12/2018 05:38, Yuya Nishihara wrote:
> > On Sat, 22 Dec 2018 13:07:25 +0900, Yuya Nishihara wrote:
> >> On Fri, 21 Dec 2018 12:47:09 +0100, Boris Feld wrote:
> >>> # HG changeset patch
> >>> # User Boris Feld <boris.feld at octobus.net>
> >>> # Date 1545040633 -3600
> >>> #      Mon Dec 17 10:57:13 2018 +0100
> >>> # Node ID d15f3f46ca1e52f06d5b8f3f77c5e2e02912aaa9
> >>> # Parent  4067f496c37ec6d10860504186917bb6be83db46
> >>> # EXP-Topic sparse-revlog
> >>> # Available At https://bitbucket.org/octobus/mercurial-devel/
> >>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r d15f3f46ca1e
> >>> revlog: add a native implementation of issnapshot
> >>> +	return index_issnapshotrev(self, base);
> >> Maybe you missed the comment in the previous series, but this function will
> >> never stop if base goes e.g. upwards at some point, and SEGV. Appears that
> >> gcc can turn the recursion into loop in this case, but still there's a risk
> >> of infinite loop.
> > To be clear, I mean
> >
> >  a. the recursion should be rewritten as a simple loop
> >  b. and the loop needs upper limit (e.g. i < rev + 1) to make sure it
> >     terminates
> I feel like any method using `index_baserev` could be affected (eg:
> index_deltachain). I am updating the code to have `index_baserev` return
> an error if it goes upward.

Good catch.

Please also rewrite the recursion as a loop for clarity. I don't think the
stack would be fully consumed in practice, but that's technically possible
with extremely long chain.


More information about the Mercurial-devel mailing list