Mercurial > hg
changeset 33174:f4f52bb362e6
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
| author | Gregory Szorc <gregory.szorc@gmail.com> |
|---|---|
| date | Sat, 01 Jul 2017 19:35:17 -0700 |
| parents | 4b0da963586d |
| children | 3b85c474cbcf |
| files | mercurial/cext/revlog.c |
| diffstat | 1 files changed, 15 insertions(+), 7 deletions(-) [+] |
line wrap: on
line diff
--- 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; }