D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGfa33196088c4: revlog: replace PyInt_AS_LONG with a more
portable helper function (authored by durin42, committed by ).
CHANGED PRIOR TO COMMIT
https://phab.mercurial-scm.org/D5235?vs=12502=12509#toc
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D5235?vs=12502=12509
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/cext/util.h
CHANGE DETAILS
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
@@ -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), )) {
+ return -1;
+ }
+ ps[0] = (int)tmp;
+ if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 6), )) {
+ return -1;
+ }
+ ps[1] = (int)tmp;
} else {
const char *data = index_deref(self, rev);
ps[0] = getbe32(data + 24);
@@ -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, _item_long)) {
+ Py_DECREF(iter_item);
+ return -2;
+ }
Py_DECREF(iter_item);
if (iter_item_long < min_idx)
min_idx = iter_item_long;
@@ -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), )) {
+ return -2;
+ }
+ return (int)ret;
} else {
data = index_deref(self, rev);
if (data == NULL) {
@@ -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, )) {
+ return NULL;
+ }
+ return index_get(self, idx);
+ }
if (node_check(value, ) == -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, )) {
+ return -1;
+ }
return rev >= -1 && rev < index_length(self);
}
@@ -2404,10 +2425,12 @@
static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
{
- if (!(PyInt_Check(rev))) {
+ long lrev;
+ if (!pylong_to_long(rev, )) {
+ PyErr_Clear();
return 0;
}
- return rustlazyancestors_contains(self->iter, PyInt_AS_LONG(rev));
+ return rustlazyancestors_contains(self->iter, lrev);
}
static PySequenceMethods rustla_sequence_methods = {
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
durin42 updated this revision to Diff 12502.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D5235?vs=12469=12502
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/cext/util.h
CHANGE DETAILS
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), )) {
+ return -1;
+ }
+ ps[0] = (int)tmp;
+ if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 6), )) {
+ 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, _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!", , _Type, ,
- _Type, , _Type,
- ))
+ _Type, , _Type,
+ ))
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;
}
D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
yuja added a comment.
> @@ -464,7 +470,9 @@
>
> 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, _item_long)) { +
return -1;
`Py_DECREF(iter_item)` and `return -2`.
I didn't notice it last time, sorry.
> + }
>
> Py_DECREF(iter_item);
> if (iter_item_long < min_idx)
> min_idx = iter_item_long;
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
> @@ -464,7 +470,9 @@
> 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, _item_long)) {
> + return -1;
`Py_DECREF(iter_item)` and `return -2`.
I didn't notice it last time, sorry.
> + }
> Py_DECREF(iter_item);
> if (iter_item_long < min_idx)
> min_idx = iter_item_long;
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
durin42 added a comment.
In https://phab.mercurial-scm.org/D5235#78172, @yuja wrote:
> > +/* 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)
>
> Nit: I prefer 0/-1 return value instead of bool since that's the convention
> of CPython API.
I got lazy, but we can change it if we like.
>
>
>> + *out = PyLong_AsLong(pylong);
>> + return PyErr_Occurred() == NULL;
>
> Perhaps, checking `*out != -1` can be a fast path, though I don't know how
> much we care about the performance here.
I don't either, but it was easy to add the if statement.
>
>
>> static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
>> {
>>
>> - if (!(PyInt_Check(rev))) { + long lrev; +if
(!pylong_to_long(rev, )) { return 0;
>
> Needs `PyErr_Clear() since `non_int in self` isn't an error.
Good catch, fixed.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
durin42 updated this revision to Diff 12469.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D5235?vs=12458=12469
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/cext/util.h
CHANGE DETAILS
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
@@ -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), )) {
+ return -1;
+ }
+ ps[0] = (int)tmp;
+ if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 6), )) {
+ return -1;
+ }
+ ps[1] = (int)tmp;
} else {
const char *data = index_deref(self, rev);
ps[0] = getbe32(data + 24);
@@ -464,7 +470,9 @@
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, _item_long)) {
+ return -1;
+ }
Py_DECREF(iter_item);
if (iter_item_long < min_idx)
min_idx = iter_item_long;
@@ -853,7 +861,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), )) {
+ return -2;
+ }
+ return (int)ret;
} else {
data = index_deref(self, rev);
if (data == NULL) {
@@ -1384,8 +1396,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, )) {
+ return NULL;
+ }
+ return index_get(self, idx);
+ }
if (node_check(value, ) == -1)
return NULL;
@@ -1516,7 +1533,10 @@
char *node;
if (PyInt_Check(value)) {
- long rev = PyInt_AS_LONG(value);
+ long rev;
+ if (!pylong_to_long(value, )) {
+ return -1;
+ }
return rev >= -1 && rev < index_length(self);
}
@@ -2404,10 +2424,12 @@
static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
{
- if (!(PyInt_Check(rev))) {
+ long lrev;
+ if (!pylong_to_long(rev, )) {
+ PyErr_Clear();
return 0;
}
- return rustlazyancestors_contains(self->iter, PyInt_AS_LONG(rev));
+ return rustlazyancestors_contains(self->iter, lrev);
}
static PySequenceMethods rustla_sequence_methods = {
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
yuja added a comment.
> +/* 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)
Nit: I prefer 0/-1 return value instead of bool since that's the convention
of CPython API.
> + *out = PyLong_AsLong(pylong);
> + return PyErr_Occurred() == NULL;
Perhaps, checking `*out != -1` can be a fast path, though I don't know how
much we care about the performance here.
> static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
> {
>
> - if (!(PyInt_Check(rev))) { + long lrev; +if
(!pylong_to_long(rev, )) { return 0;
Needs `PyErr_Clear() since `non_int in self` isn't an error.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
> +/* 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)
Nit: I prefer 0/-1 return value instead of bool since that's the convention
of CPython API.
> + *out = PyLong_AsLong(pylong);
> + return PyErr_Occurred() == NULL;
Perhaps, checking `*out != -1` can be a fast path, though I don't know how
much we care about the performance here.
> static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
> {
> - if (!(PyInt_Check(rev))) {
> + long lrev;
> + if (!pylong_to_long(rev, )) {
> return 0;
Needs `PyErr_Clear() since `non_int in self` isn't an error.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5235: revlog: replace PyInt_AS_LONG with a more portable helper function
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
PyInt_AS_LONG disappears on Python, and our previous #define was
producing some problems on Python 3. Let's give up and make an inline
helper function that makes this more sane.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5235
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/cext/util.h
CHANGE DETAILS
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,12 @@
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);
+ 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
@@ -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), )) {
+ return -1;
+ }
+ ps[0] = (int)tmp;
+ if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 6), )) {
+ return -1;
+ }
+ ps[1] = (int)tmp;
} else {
const char *data = index_deref(self, rev);
ps[0] = getbe32(data + 24);
@@ -464,7 +470,9 @@
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, _item_long)) {
+ return -1;
+ }
Py_DECREF(iter_item);
if (iter_item_long < min_idx)
min_idx = iter_item_long;
@@ -853,7 +861,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), )) {
+ return -2;
+ }
+ return (int)ret;
} else {
data = index_deref(self, rev);
if (data == NULL) {
@@ -1384,8 +1396,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, )) {
+ return NULL;
+ }
+ return index_get(self, idx);
+ }
if (node_check(value, ) == -1)
return NULL;
@@ -1516,7 +1533,10 @@
char *node;
if (PyInt_Check(value)) {
- long rev = PyInt_AS_LONG(value);
+ long rev;
+ if (!pylong_to_long(value, )) {
+ return -1;
+ }
return rev >= -1 && rev < index_length(self);
}
@@ -2404,10 +2424,11 @@
static int rustla_contains(rustlazyancestorsObject *self, PyObject *rev)
{
- if (!(PyInt_Check(rev))) {
+ long lrev;
+ if (!pylong_to_long(rev, )) {
return 0;
}
- return rustlazyancestors_contains(self->iter, PyInt_AS_LONG(rev));
+ return rustlazyancestors_contains(self->iter, lrev);
}
static PySequenceMethods rustla_sequence_methods = {
To: durin42, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
