This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12376

revlog: fix index_fast_rank (wip)
ClosedPublic

Authored by jcristau on Mar 15 2022, 5:38 AM.

Details

Reviewers
Alphare
pacien
Group Reviewers
hg-reviewers
Commits
rHG5ade9023beb7: revlog: fix index_fast_rank
rHG890db1c97218: revlog: fix index_fast_rank (wip)
Summary

As far as I can tell, rank is stored as a 32-bit big endian value, I'm
not sure how grabbing the first byte can possibly work. I assume
there's no test coverage here?

cc @pacien

Fixes: https://www.mercurial-scm.org/repo/hg/rev/e633e660158f

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jcristau created this revision.Mar 15 2022, 5:38 AM
baymax updated this revision to Diff 32645.Mar 15 2022, 6:58 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare added a subscriber: Alphare.Mar 15 2022, 7:09 AM

FYI I took the liberty to send this to the CI to confirm this either works or is part of code that is not covered. I'll wait for @pacien to answer

Alphare accepted this revision.Mar 17 2022, 5:13 AM

I'll amend the title in-flight. This is covered (at least) by test-rank.t, maybe it "works" because we're only testing on little endian machines and the values never exceed 255?

This revision is now accepted and ready to land.Mar 17 2022, 5:13 AM
pacien accepted this revision.Mar 17 2022, 5:28 AM
jcristau added a commit: rHG890db1c97218: revlog: fix index_fast_rank (wip).Mar 17 2022, 7:44 AM
Closed by commit rHG890db1c97218: revlog: fix index_fast_rank (wip) (authored by jcristau). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/revlog.c (2 lines)

Diff 32657

mercurial/cext/revlog.c

if (self->format_version != format_cl2 || pos >= length) { if (self->format_version != format_cl2 || pos >= length) {
return rank_unknown; return rank_unknown;
} }
if (pos == nullrev) { if (pos == nullrev) {
return 0; /* convention */ return 0; /* convention */
} }
return *(index_deref(self, pos) + entry_cl2_offset_rank); return getbe32(index_deref(self, pos) + entry_cl2_offset_rank);
} }
/* /*
* Return the hash of the node corresponding to the given rev. The * Return the hash of the node corresponding to the given rev. The
* rev is assumed to be existing. If not, an exception is set. * rev is assumed to be existing. If not, an exception is set.
*/ */
static const char *index_node_existing(indexObject *self, Py_ssize_t pos) static const char *index_node_existing(indexObject *self, Py_ssize_t pos)
{ {