Patchwork D5235: revlog: replace PyInt_AS_LONG with a more portable helper function

login
register
mail settings
Submitter phabricator
Date Nov. 12, 2018, 4:46 p.m.
Message ID <61b821931e75336f70cca17de1cc44c1@localhost.localdomain>
Download mbox | patch
Permalink /patch/36527/
State Not Applicable
Headers show

Comments

phabricator - Nov. 12, 2018, 4:46 p.m.
durin42 updated this revision to Diff 12502.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5235?vs=12469&id=12502

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

AFFECTED FILES
  mercurial/cext/revlog.c
  mercurial/cext/util.h

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/cext/util.h b/mercurial/cext/util.h
--- a/mercurial/cext/util.h
+++ b/mercurial/cext/util.h
@@ -58,4 +58,17 @@ 
 	return _PyDict_NewPresized(((1 + expected_size) / 2) * 3);
 }
 
+/* Convert a PyInt or PyLong to a long. Returns false if there is an
+   error, in which case an exception will already have been set. */
+static inline bool pylong_to_long(PyObject *pylong, long *out)
+{
+	*out = PyLong_AsLong(pylong);
+	/* Fast path to avoid hitting PyErr_Occurred if the value was obviously
+	 * not an error. */
+	if (*out != -1) {
+		return true;
+	}
+	return PyErr_Occurred() == NULL;
+}
+
 #endif /* _HG_UTIL_H_ */
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -24,7 +24,6 @@ 
 #define PyInt_Check PyLong_Check
 #define PyInt_FromLong PyLong_FromLong
 #define PyInt_FromSsize_t PyLong_FromSsize_t
-#define PyInt_AS_LONG PyLong_AS_LONG
 #define PyInt_AsLong PyLong_AsLong
 #endif
 
@@ -126,7 +125,7 @@ 
 	errclass = PyDict_GetItemString(dict, "RevlogError");
 	if (errclass == NULL) {
 		PyErr_SetString(PyExc_SystemError,
-		                "could not find RevlogError");
+				"could not find RevlogError");
 		goto cleanup;
 	}
 
@@ -146,7 +145,7 @@ 
 	if (self->inlined && pos > 0) {
 		if (self->offsets == NULL) {
 			self->offsets = PyMem_Malloc(self->raw_length *
-			                             sizeof(*self->offsets));
+						     sizeof(*self->offsets));
 			if (self->offsets == NULL)
 				return (const char *)PyErr_NoMemory();
 			inline_scan(self, self->offsets);
@@ -161,10 +160,17 @@ 
                                     int maxrev)
 {
 	if (rev >= self->length) {
+		long tmp;
 		PyObject *tuple =
 		    PyList_GET_ITEM(self->added, rev - self->length);
-		ps[0] = (int)PyInt_AS_LONG(PyTuple_GET_ITEM(tuple, 5));
-		ps[1] = (int)PyInt_AS_LONG(PyTuple_GET_ITEM(tuple, 6));
+		if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 5), &tmp)) {
+			return -1;
+		}
+		ps[0] = (int)tmp;
+		if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 6), &tmp)) {
+			return -1;
+		}
+		ps[1] = (int)tmp;
 	} else {
 		const char *data = index_deref(self, rev);
 		ps[0] = getbe32(data + 24);
@@ -249,8 +255,8 @@ 
 	c_node_id = data + 32;
 
 	entry = Py_BuildValue(tuple_format, offset_flags, comp_len, uncomp_len,
-	                      base_rev, link_rev, parent_1, parent_2, c_node_id,
-	                      20);
+			      base_rev, link_rev, parent_1, parent_2, c_node_id,
+			      20);
 
 	if (entry) {
 		PyObject_GC_UnTrack(entry);
@@ -296,7 +302,7 @@ 
 	const char *node = index_node(self, pos);
 	if (node == NULL) {
 		PyErr_Format(PyExc_IndexError, "could not access rev %d",
-		             (int)pos);
+			     (int)pos);
 	}
 	return node;
 }
@@ -464,7 +470,10 @@ 
 		if (iter == NULL)
 			return -2;
 		while ((iter_item = PyIter_Next(iter))) {
-			iter_item_long = PyInt_AS_LONG(iter_item);
+			if (!pylong_to_long(iter_item, &iter_item_long)) {
+				Py_DECREF(iter_item);
+				return -2;
+			}
 			Py_DECREF(iter_item);
 			if (iter_item_long < min_idx)
 				min_idx = iter_item_long;
@@ -518,8 +527,8 @@ 
 
 	/* Get arguments */
 	if (!PyArg_ParseTuple(args, "lO!O!O!", &minroot, &PyList_Type, &heads,
-	                      &PyList_Type, &roots, &PyBool_Type,
-	                      &includepatharg))
+			      &PyList_Type, &roots, &PyBool_Type,
+			      &includepatharg))
 		goto bail;
 
 	if (includepatharg == Py_True)
@@ -692,7 +701,7 @@ 
 		PyList_SET_ITEM(phasessetlist, i + 1, phaseset);
 		if (!PyList_Check(phaseroots)) {
 			PyErr_SetString(PyExc_TypeError,
-			                "roots item must be a list");
+					"roots item must be a list");
 			goto release;
 		}
 		minrevphase =
@@ -709,7 +718,7 @@ 
 			    0)
 				goto release;
 			set_phase_from_parents(phases, parents[0], parents[1],
-			                       i);
+					       i);
 		}
 	}
 	/* Transform phase list to a python list */
@@ -799,7 +808,7 @@ 
 			isfiltered = check_filter(filter, i);
 			if (isfiltered == -1) {
 				PyErr_SetString(PyExc_TypeError,
-				                "unable to check filter");
+						"unable to check filter");
 				goto bail;
 			}
 
@@ -853,7 +862,11 @@ 
 	if (rev >= self->length) {
 		PyObject *tuple =
 		    PyList_GET_ITEM(self->added, rev - self->length);
-		return (int)PyInt_AS_LONG(PyTuple_GET_ITEM(tuple, 3));
+		long ret;
+		if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 3), &ret)) {
+			return -2;
+		}
+		return (int)ret;
 	} else {
 		data = index_deref(self, rev);
 		if (data == NULL) {
@@ -886,7 +899,7 @@ 
 		stoprev = -2;
 	} else {
 		PyErr_SetString(PyExc_ValueError,
-		                "stoprev must be integer or None");
+				"stoprev must be integer or None");
 		return NULL;
 	}
 
@@ -934,7 +947,7 @@ 
 
 		if (iterrev >= length) {
 			PyErr_SetString(PyExc_IndexError,
-			                "revision outside index");
+					"revision outside index");
 			return NULL;
 		}
 
@@ -1040,7 +1053,7 @@ 
 		newcapacity = self->capacity * 2;
 		if (newcapacity >= INT_MAX / sizeof(nodetreenode)) {
 			PyErr_SetString(PyExc_MemoryError,
-			                "overflow in nt_new");
+					"overflow in nt_new");
 			return -1;
 		}
 		newnodes =
@@ -1384,8 +1397,13 @@ 
 	char *node;
 	int rev;
 
-	if (PyInt_Check(value))
-		return index_get(self, PyInt_AS_LONG(value));
+	if (PyInt_Check(value)) {
+		long idx;
+		if (!pylong_to_long(value, &idx)) {
+			return NULL;
+		}
+		return index_get(self, idx);
+	}
 
 	if (node_check(value, &node) == -1)
 		return NULL;
@@ -1516,7 +1534,10 @@ 
 	char *node;
 
 	if (PyInt_Check(value)) {
-		long rev = PyInt_AS_LONG(value);
+		long rev;
+		if (!pylong_to_long(value, &rev)) {
+			return -1;
+		}
 		return rev >= -1 && rev < index_length(self);
 	}
 
@@ -1642,8 +1663,8 @@ 
 
 	if (revcount > capacity) {
 		PyErr_Format(PyExc_OverflowError,
-		             "bitset size (%ld) > capacity (%ld)",
-		             (long)revcount, (long)capacity);
+			     "bitset size (%ld) > capacity (%ld)",
+			     (long)revcount, (long)capacity);
 		return NULL;
 	}
 
@@ -1805,7 +1826,7 @@ 
 
 		if (!PyInt_Check(obj)) {
 			PyErr_SetString(PyExc_TypeError,
-			                "arguments must all be ints");
+					"arguments must all be ints");
 			Py_DECREF(obj);
 			goto bail;
 		}
@@ -1833,8 +1854,8 @@ 
 			repeat |= x;
 		if (revcount >= capacity) {
 			PyErr_Format(PyExc_OverflowError,
-			             "bitset size (%d) > capacity (%d)",
-			             revcount, capacity);
+				     "bitset size (%d) > capacity (%d)",
+				     revcount, capacity);
 			goto bail;
 		}
 		revs[revcount++] = (int)val;
@@ -1922,10 +1943,10 @@ 
 /* Argument changed from PySliceObject* to PyObject* in Python 3. */
 #ifdef IS_PY3K
 	if (PySlice_GetIndicesEx(item, length, &start, &stop, &step,
-	                         &slicelength) < 0)
+				 &slicelength) < 0)
 #else
 	if (PySlice_GetIndicesEx((PySliceObject *)item, length, &start, &stop,
-	                         &step, &slicelength) < 0)
+				 &step, &slicelength) < 0)
 #endif
 		return -1;
 
@@ -1943,13 +1964,13 @@ 
 
 	if (step != 1) {
 		PyErr_SetString(PyExc_ValueError,
-		                "revlog index delete requires step size of 1");
+				"revlog index delete requires step size of 1");
 		return -1;
 	}
 
 	if (stop != length - 1) {
 		PyErr_SetString(PyExc_IndexError,
-		                "revlog index deletion indices are invalid");
+				"revlog index deletion indices are invalid");
 		return -1;
 	}
 
@@ -1988,7 +2009,7 @@ 
 	}
 	if (self->added)
 		ret = PyList_SetSlice(self->added, start - self->length,
-		                      PyList_GET_SIZE(self->added), NULL);
+				      PyList_GET_SIZE(self->added), NULL);
 done:
 	Py_CLEAR(self->headrevs);
 	return ret;
@@ -2015,7 +2036,7 @@ 
 
 	if (value == NULL)
 		return self->ntinitialized ? nt_delete_node(&self->nt, node)
-		                           : 0;
+					   : 0;
 	rev = PyInt_AsLong(value);
 	if (rev > INT_MAX || rev < 0) {
 		if (!PyErr_Occurred())
@@ -2082,7 +2103,7 @@ 
 		return -1;
 	if (!PyObject_CheckBuffer(data_obj)) {
 		PyErr_SetString(PyExc_TypeError,
-		                "data does not support buffer interface");
+				"data does not support buffer interface");
 		return -1;
 	}
 
@@ -2339,8 +2360,8 @@ 
 
 	indexObject *index;
 	if (!PyArg_ParseTuple(args, "O!O!lO!", &indexType, &index, &PyList_Type,
-	                      &initrevsarg, &stoprev, &PyBool_Type,
-	                      &inclusivearg))
+			      &initrevsarg, &stoprev, &PyBool_Type,
+			      &inclusivearg))
 		return -1;
 
 	Py_INCREF(index);
@@ -2365,7 +2386,7 @@ 
 		goto bail;
 
 	self->iter = rustlazyancestors_init(index, index_get_parents, linit,
-	                                    initrevs, stoprev, inclusive);
+					    initrevs, stoprev, inclusive);
 	if (self->iter == NULL) {
 		/* if this is because of GraphError::ParentOutOfRange
 		 * index_get_parents() has already set the proper ValueError */
@@ -2404,10 +2425,12 @@ 
 
 static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
 {
-	if (!(PyInt_Check(rev))) {
+	long lrev;
+	if (!pylong_to_long(rev, &lrev)) {
+		PyErr_Clear();
 		return 0;
 	}
-	return rustlazyancestors_contains(self->iter, PyInt_AS_LONG(rev));
+	return rustlazyancestors_contains(self->iter, lrev);
 }
 
 static PySequenceMethods rustla_sequence_methods = {
@@ -2478,7 +2501,7 @@ 
 
 	if (!nullentry) {
 		nullentry = Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0,
-		                          0, -1, -1, -1, -1, nullid, 20);
+					  0, -1, -1, -1, -1, nullid, 20);
 	}
 	if (nullentry)
 		PyObject_GC_UnTrack(nullentry);
@@ -2489,6 +2512,6 @@ 
 		return;
 	Py_INCREF(&rustlazyancestorsType);
 	PyModule_AddObject(mod, "rustlazyancestors",
-	                   (PyObject *)&rustlazyancestorsType);
+			   (PyObject *)&rustlazyancestorsType);
 #endif
 }