This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3457

revlog: don't say "not found" on internal error
ClosedPublic

Authored by martinvonz on May 7 2018, 12:55 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGa9d9802d577e: revlog: don't say "not found" on internal error
Summary

If index_node() returned NULL, then index_find_node() and and
nt_partialmatch() used to return -2 to signal that the node was not
found. However, we were passing in a revnum to index_node() that we
knew should exist, so the only reason it could return NULL was due to
some internal error or perhaps out of memory. Let's not use "not
found" for these cases. I suppose we never noticed this because these
error never happen in practice.

I think there are more places where we should error out instead of
reporting that the node was not found, but the cases mentioned above
were all I cared about right now (because using the same error code
for all failures simplified some future patches).

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

martinvonz created this revision.May 7 2018, 12:55 PM
Closed by commit rHGa9d9802d577e: revlog: don't say "not found" on internal error (authored by martinvonz). · Explain WhyMay 8 2018, 8:47 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

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

Diff 8511

mercurial/cext/revlog.c

* like "hg tip", where only a few nodes are accessed. * like "hg tip", where only a few nodes are accessed.
* *
* After that, we cache every node we visit, using a single * After that, we cache every node we visit, using a single
* scan amortized over multiple lookups. This gives the best * scan amortized over multiple lookups. This gives the best
* bulk performance, e.g. for "hg log". * bulk performance, e.g. for "hg log".
*/ */
if (self->ntmisses++ < 4) { if (self->ntmisses++ < 4) {
for (rev = self->ntrev - 1; rev >= 0; rev--) { for (rev = self->ntrev - 1; rev >= 0; rev--) {
const char *n = index_node(self, rev); const char *n = index_node_existing(self, rev);
if (n == NULL) if (n == NULL)
return -2; return -3;
if (memcmp(node, n, nodelen > 20 ? 20 : nodelen) == 0) { if (memcmp(node, n, nodelen > 20 ? 20 : nodelen) == 0) {
if (nt_insert(self, n, rev) == -1) if (nt_insert(self, n, rev) == -1)
return -3; return -3;
break; break;
} }
} }
} else { } else {
for (rev = self->ntrev - 1; rev >= 0; rev--) { for (rev = self->ntrev - 1; rev >= 0; rev--) {
const char *n = index_node(self, rev); const char *n = index_node_existing(self, rev);
if (n == NULL) { if (n == NULL)
self->ntrev = rev + 1; return -3;
return -2;
}
if (nt_insert(self, n, rev) == -1) { if (nt_insert(self, n, rev) == -1) {
self->ntrev = rev + 1; self->ntrev = rev + 1;
return -3; return -3;
} }
if (memcmp(node, n, nodelen > 20 ? 20 : nodelen) == 0) { if (memcmp(node, n, nodelen > 20 ? 20 : nodelen) == 0) {
break; break;
} }
} }
int rev; int rev;
if (nt_init(self) == -1) if (nt_init(self) == -1)
return -3; return -3;
if (self->ntrev > 0) { if (self->ntrev > 0) {
/* ensure that the radix tree is fully populated */ /* ensure that the radix tree is fully populated */
for (rev = self->ntrev - 1; rev >= 0; rev--) { for (rev = self->ntrev - 1; rev >= 0; rev--) {
const char *n = index_node(self, rev); const char *n = index_node_existing(self, rev);
if (n == NULL) if (n == NULL)
return -2; return -3;
if (nt_insert(self, n, rev) == -1) if (nt_insert(self, n, rev) == -1)
return -3; return -3;
} }
self->ntrev = rev; self->ntrev = rev;
} }
return nt_find(self, node, nodelen, 1); return nt_find(self, node, nodelen, 1);
} }