Patchwork dirs: document Py_SIZE weirdness

login
register
mail settings
Submitter Gregory Szorc
Date Oct. 13, 2016, 7:43 p.m.
Message ID <c05716f3eb0a4a70ef1f.1476387820@gps-mbp.local>
Download mbox | patch
Permalink /patch/17058/
State Accepted
Headers show

Comments

Gregory Szorc - Oct. 13, 2016, 7:43 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1475939263 -7200
#      Sat Oct 08 17:07:43 2016 +0200
# Node ID c05716f3eb0a4a70ef1f59b51cd1b29ee683a145
# Parent  c0410814002f467c24ef07ce73850ba15b306f8e
dirs: document Py_SIZE weirdness

Assigning to what looks like a function is clown shoes. Document that
it is a macro referring to a struct member.
Pierre-Yves David - Oct. 14, 2016, 11:19 a.m.
On 10/13/2016 09:43 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1475939263 -7200
> #      Sat Oct 08 17:07:43 2016 +0200
> # Node ID c05716f3eb0a4a70ef1f59b51cd1b29ee683a145
> # Parent  c0410814002f467c24ef07ce73850ba15b306f8e
> dirs: document Py_SIZE weirdness
>
> Assigning to what looks like a function is clown shoes. Document that
> it is a macro referring to a struct member.

Pushed, thanks.
Maciej Fijalkowski - Oct. 14, 2016, 12:42 p.m.
FYI This is the violation of C API, explicit assignment would be
probably better (at least it's CLEAR that you're doing something
dodgy)

On Thu, Oct 13, 2016 at 9:43 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1475939263 -7200
> #      Sat Oct 08 17:07:43 2016 +0200
> # Node ID c05716f3eb0a4a70ef1f59b51cd1b29ee683a145
> # Parent  c0410814002f467c24ef07ce73850ba15b306f8e
> dirs: document Py_SIZE weirdness
>
> Assigning to what looks like a function is clown shoes. Document that
> it is a macro referring to a struct member.
>
> diff --git a/mercurial/dirs.c b/mercurial/dirs.c
> --- a/mercurial/dirs.c
> +++ b/mercurial/dirs.c
> @@ -75,8 +75,10 @@ static int _addpath(PyObject *dirs, PyOb
>                                                          pos < 2 ? 2 : pos);
>                         if (key == NULL)
>                                 goto bail;
>                 }
> +               /* Py_SIZE(o) refers to the ob_size member of the struct. Yes,
> +               * assigning to what looks like a function seems wrong. */
>                 Py_SIZE(key) = pos;
>                 ((PyBytesObject *)key)->ob_sval[pos] = '\0';
>
>                 val = PyDict_GetItem(dirs, key);
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/dirs.c b/mercurial/dirs.c
--- a/mercurial/dirs.c
+++ b/mercurial/dirs.c
@@ -75,8 +75,10 @@  static int _addpath(PyObject *dirs, PyOb
 							 pos < 2 ? 2 : pos);
 			if (key == NULL)
 				goto bail;
 		}
+		/* Py_SIZE(o) refers to the ob_size member of the struct. Yes,
+		* assigning to what looks like a function seems wrong. */
 		Py_SIZE(key) = pos;
 		((PyBytesObject *)key)->ob_sval[pos] = '\0';
 
 		val = PyDict_GetItem(dirs, key);