( )⚙ D4003 index: write expression for encoded revision index consistently

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4003

index: write expression for encoded revision index consistently
ClosedPublic

Authored by martinvonz on Aug 1 2018, 12:43 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGec76d42da32d: index: write expression for encoded revision index consistently
rHG37161dc600c5: index: write expression for encoded revision index consistently
Summary

It was sometimes "-(v + 1)" and sometimes "-v - 1" before this patch.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Aug 1 2018, 12:43 PM
pulkit accepted this revision.Aug 1 2018, 4:48 PM
Closed by commit rHG37161dc600c5: index: write expression for encoded revision index consistently (authored by martinvonz). · Explain WhyAug 1 2018, 5:03 PM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Aug 2 2018, 8:57 AM
  • n->children[k] = -rev - 1;

+ n->children[k] = -(rev + 1);

This is technically less correct since (MAX_INT + 1) overflows, but maybe
we don't care?

martinvonz added a comment.Aug 2 2018, 10:46 AM
In D4003#62821, @yuja wrote:
  • n->children[k] = -rev - 1;

+ n->children[k] = -(rev + 1);

This is technically less correct since (MAX_INT + 1) overflows, but maybe
we don't care?

Good point, I had not noticed before that the encoding was using one form and the decoding was using the other and the reason was almost certainly to avoid integer overflow. It won't be relevant after D4005, but I'll dequeue this patch and update D4004 and D4005.

Revision Contents
Changeset List

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

Diff 9695

mercurial/cext/revlog.c

int k = nt_level(node, level); int k = nt_level(node, level);
nodetree *n; nodetree *n;
int v; int v;
n = &self->nt[off]; n = &self->nt[off];
v = n->children[k]; v = n->children[k];
if (v == 0) { if (v == 0) {
n->children[k] = -rev - 1; n->children[k] = -(rev + 1);
return 0; return 0;
} }
if (v < 0) { if (v < 0) {
const char *oldnode = index_node_existing(self, -(v + 1)); const char *oldnode = index_node_existing(self, -(v + 1));
int noff; int noff;
if (oldnode == NULL) if (oldnode == NULL)
return -1; return -1;
if (!memcmp(oldnode, node, 20)) { if (!memcmp(oldnode, node, 20)) {
n->children[k] = -rev - 1; n->children[k] = -(rev + 1);
return 0; return 0;
} }
noff = nt_new(self); noff = nt_new(self);
if (noff == -1) if (noff == -1)
return -1; return -1;
/* self->nt may have been changed by realloc */ /* self->nt may have been changed by realloc */
self->nt[off].children[k] = noff; self->nt[off].children[k] = noff;
off = noff; off = noff;