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

login
register
mail settings
Submitter phabricator
Date May 7, 2018, 4:55 p.m.
Message ID <differential-rev-PHID-DREV-msye5bgnsarb542yoo5s-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/31308/
State Superseded
Headers show

Comments

phabricator - May 7, 2018, 4:55 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3457

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -1149,22 +1149,20 @@ 
 	 */
 	if (self->ntmisses++ < 4) {
 		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)
-				return -2;
+				return -3;
 			if (memcmp(node, n, nodelen > 20 ? 20 : nodelen) == 0) {
 				if (nt_insert(self, n, rev) == -1)
 					return -3;
 				break;
 			}
 		}
 	} else {
 		for (rev = self->ntrev - 1; rev >= 0; rev--) {
-			const char *n = index_node(self, rev);
-			if (n == NULL) {
-				self->ntrev = rev + 1;
-				return -2;
-			}
+			const char *n = index_node_existing(self, rev);
+			if (n == NULL)
+				return -3;
 			if (nt_insert(self, n, rev) == -1) {
 				self->ntrev = rev + 1;
 				return -3;
@@ -1241,9 +1239,9 @@ 
 	if (self->ntrev > 0) {
 		/* ensure that the radix tree is fully populated */
 		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)
-				return -2;
+				return -3;
 			if (nt_insert(self, n, rev) == -1)
 				return -3;
 		}