Patchwork D6196: cext: make revlog.c PY_SSIZE_T_CLEAN

login
register
mail settings
Submitter phabricator
Date April 4, 2019, 10:31 p.m.
Message ID <differential-rev-PHID-DREV-g2iptmygyxfqinrpuwt4-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39481/
State Superseded
Headers show

Comments

phabricator - April 4, 2019, 10:31 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Without this, Python 3.8 emits a deprecation warning, as using
  int for # values is deprecated. Many existing modules use
  PY_SSIZE_T_CLEAN, so this shouldn't be contentious.
  
  I audited the file for all # formatters and verified we are
  using Py_ssize_t everywhere now.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 9, 2019, 4:51 p.m.
mharbison72 added a comment.


  In https://phab.mercurial-scm.org/D6196#90521, @yuja wrote:
  
  > >   4   org.python.python             	0x00000001000eb761 va_build_value + 737
  > >   5   org.python.python             	0x00000001000eb837 _Py_BuildValue_SizeT + 167
  >
  > Maybe we shouldn't trust the Python doc too much, which says 's#' of
  >  Py_BuildValue() takes an int.
  >
  > https://docs.python.org/2/c-api/arg.html#c.Py_BuildValue
  >
  > Can you test if this patch fixes the problem?
  >
  >   diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
  >   --- a/mercurial/cext/parsers.c
  >   +++ b/mercurial/cext/parsers.c
  >   @@ -184,7 +184,8 @@ static PyObject *parse_dirstate(PyObject
  >    		goto quit;
  >    	}
  >   
  >   -	parents = Py_BuildValue(PY23("s#s#", "y#y#"), str, 20, str + 20, 20);
  >   +	parents = Py_BuildValue(PY23("s#s#", "y#y#"), str, (Py_ssize_t)20,
  >   +	                        str + 20, (Py_ssize_t)20);
  >    	if (!parents) {
  >    		goto quit;
  >    	}
  >   diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
  >   --- a/mercurial/cext/revlog.c
  >   +++ b/mercurial/cext/revlog.c
  >   @@ -366,7 +366,7 @@ static PyObject *index_get(indexObject *
  >   
  >    	entry = Py_BuildValue(tuple_format, offset_flags, comp_len, uncomp_len,
  >    	                      base_rev, link_rev, parent_1, parent_2, c_node_id,
  >   -	                      20);
  >   +	                      (Py_ssize_t)20);
  >   
  >    	if (entry) {
  >    		PyObject_GC_UnTrack(entry);
  >   @@ -3017,8 +3017,9 @@ void revlog_module_init(PyObject *mod)
  >    	PyModule_AddObject(mod, "nodetree", (PyObject *)&nodetreeType);
  >   
  >    	if (!nullentry) {
  >   -		nullentry = Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0,
  >   -		                          0, -1, -1, -1, -1, nullid, 20);
  >   +		nullentry =
  >   +		    Py_BuildValue(PY23("iiiiiiis#", "iiiiiiiy#"), 0, 0, 0, -1,
  >   +		                  -1, -1, -1, nullid, (Py_ssize_t)20);
  >    	}
  >    	if (nullentry)
  >    		PyObject_GC_UnTrack(nullentry);
  >
  
  
  That cleared the whole test suite, thanks.

REPOSITORY
  rHG Mercurial

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

To: indygreg, #hg-reviewers, pulkit
Cc: yuja, mharbison72, mercurial-devel

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -7,6 +7,7 @@ 
  the GNU General Public License, incorporated herein by reference.
 */
 
+#define PY_SSIZE_T_CLEAN
 #include <Python.h>
 #include <assert.h>
 #include <ctype.h>
@@ -1947,7 +1948,7 @@ 
 static PyObject *index_partialmatch(indexObject *self, PyObject *args)
 {
 	const char *fullnode;
-	int nodelen;
+	Py_ssize_t nodelen;
 	char *node;
 	int rev, i;