Patchwork [stable,v2] parsers: use 'k' format for Py_BuildValue instead of 'n' because Python 2.4

login
register
mail settings
Submitter Mads Kiilerich
Date Oct. 23, 2014, 12:43 a.m.
Message ID <5715c93cb8540dc7b6d4.1414024981@localhost.localdomain>
Download mbox | patch
Permalink /patch/6443/
State Accepted
Headers show

Comments

Mads Kiilerich - Oct. 23, 2014, 12:43 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1414024977 -7200
#      Thu Oct 23 02:42:57 2014 +0200
# Branch stable
# Node ID 5715c93cb8540dc7b6d4dc2d0fde07dfd0ba9033
# Parent  d583f1cfca9670946d7e315df4d0a0efccb7a612
parsers: use 'k' format for Py_BuildValue instead of 'n' because Python 2.4

'n' was introduced in Mercurial in 2b5940f64750 and broke Python 2.4 support in
mysterious ways that only showed failure in test-glog.t. Py_BuildValue failed
because of the unknown format and a TypeError was thrown ... but it never
showed up on the Python side and it happily continued processing with wrong
data.

Quoting https://docs.python.org/2/c-api/arg.html :

  n (integer) [Py_ssize_t]
    Convert a Python integer or long integer to a C Py_ssize_t.
    New in version 2.5.

  k (integer) [unsigned long]
    Convert a Python integer or long integer to a C unsigned long without
    overflow checking.

This will use unsigned long instead of Py_ssize_t. That is not a good solution,
but good is not an option when we have to support Python 2.4.
Pierre-Yves David - Oct. 23, 2014, 1:29 a.m.
On 10/22/2014 05:43 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1414024977 -7200
> #      Thu Oct 23 02:42:57 2014 +0200
> # Branch stable
> # Node ID 5715c93cb8540dc7b6d4dc2d0fde07dfd0ba9033
> # Parent  d583f1cfca9670946d7e315df4d0a0efccb7a612
> parsers: use 'k' format for Py_BuildValue instead of 'n' because Python 2.4
>
> 'n' was introduced in Mercurial in 2b5940f64750 and broke Python 2.4 support in
> mysterious ways that only showed failure in test-glog.t. Py_BuildValue failed
> because of the unknown format and a TypeError was thrown ... but it never
> showed up on the Python side and it happily continued processing with wrong
> data.
>
> Quoting https://docs.python.org/2/c-api/arg.html :
>
>    n (integer) [Py_ssize_t]
>      Convert a Python integer or long integer to a C Py_ssize_t.
>      New in version 2.5.
>
>    k (integer) [unsigned long]
>      Convert a Python integer or long integer to a C unsigned long without
>      overflow checking.
>
> This will use unsigned long instead of Py_ssize_t. That is not a good solution,
> but good is not an option when we have to support Python 2.4.

This seems reasonable (lgtm) I would prefer patch1 to be sorted out 
before pushing this.
Matt Mackall - Oct. 23, 2014, 4:49 p.m.
On Thu, 2014-10-23 at 02:43 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1414024977 -7200
> #      Thu Oct 23 02:42:57 2014 +0200
> # Branch stable
> # Node ID 5715c93cb8540dc7b6d4dc2d0fde07dfd0ba9033
> # Parent  d583f1cfca9670946d7e315df4d0a0efccb7a612
> parsers: use 'k' format for Py_BuildValue instead of 'n' because Python 2.4

Queued for stable, thanks.

Patch

diff --git a/mercurial/parsers.c b/mercurial/parsers.c
--- a/mercurial/parsers.c
+++ b/mercurial/parsers.c
@@ -873,12 +873,13 @@  static PyObject *list_copy(PyObject *lis
 	return newlist;
 }
 
-static int check_filter(PyObject *filter, Py_ssize_t arg) {
+/* arg should be Py_ssize_t but Python 2.4 do not support the n format */
+static int check_filter(PyObject *filter, unsigned long arg) {
 	if (filter) {
 		PyObject *arglist, *result;
 		int isfiltered;
 
-		arglist = Py_BuildValue("(n)", arg);
+		arglist = Py_BuildValue("(k)", arg);
 		if (!arglist) {
 			return -1;
 		}