revlog: address review feedback for deltachain C implementation
* Scope of "value" is reduced
* index_baserev() is documented
* Error is no longer redundantly set for -2 return values
* Error values are compared <= -2 instead of == -2 to protect
against odd failure scenarios
--- a/mercurial/cext/revlog.c Sat Jul 01 15:13:09 2017 -0400
+++ b/mercurial/cext/revlog.c Sat Jul 01 19:35:17 2017 -0700
@@ -8,6 +8,7 @@
*/
#include <Python.h>
+#include <assert.h>
#include <ctype.h>
#include <stddef.h>
#include <string.h>
@@ -816,6 +817,11 @@
return NULL;
}
+/**
+ * Obtain the base revision index entry.
+ *
+ * Callers must ensure that rev >= 0 or illegal memory access may occur.
+ */
static inline int index_baserev(indexObject *self, int rev)
{
const char *data;
@@ -841,7 +847,7 @@
PyObject *stoparg;
int stoprev, iterrev, baserev = -1;
int stopped;
- PyObject *chain = NULL, *value = NULL, *result = NULL;
+ PyObject *chain = NULL, *result = NULL;
const Py_ssize_t length = index_length(self);
if (!PyArg_ParseTuple(args, "iOi", &rev, &stoparg, &generaldelta)) {
@@ -876,15 +882,16 @@
baserev = index_baserev(self, rev);
/* This should never happen. */
- if (baserev == -2) {
- PyErr_SetString(PyExc_IndexError, "unable to resolve data");
+ if (baserev <= -2) {
+ /* Error should be set by index_deref() */
+ assert(PyErr_Occurred());
goto bail;
}
iterrev = rev;
while (iterrev != baserev && iterrev != stoprev) {
- value = PyInt_FromLong(iterrev);
+ PyObject *value = PyInt_FromLong(iterrev);
if (value == NULL) {
goto bail;
}
@@ -913,8 +920,9 @@
baserev = index_baserev(self, iterrev);
/* This should never happen. */
- if (baserev == -2) {
- PyErr_SetString(PyExc_IndexError, "unable to resolve data");
+ if (baserev <= -2) {
+ /* Error should be set by index_deref() */
+ assert(PyErr_Occurred());
goto bail;
}
}
@@ -923,7 +931,7 @@
stopped = 1;
}
else {
- value = PyInt_FromLong(iterrev);
+ PyObject *value = PyInt_FromLong(iterrev);
if (value == NULL) {
goto bail;
}