[PATCH 0/3] trace-cmd: Update python plugin for Python 3

[PATCH 0/3] trace-cmd: Update python plugin for Python 3

[Steven Rostedt]

As reported here:

  https://bugzilla.kernel.org/show_bug.cgi?id=204231

The trace-cmd python plugin doesn't build properly for Python 3.
As Python 2 is going to be EOL soon, these should fix that. Note,
I'm not strong in using python, and these changes basically came from
googling how to handle these deprecations. Please have a look to
see if I didn't break anything.

Steven Rostedt (VMware) (3):
      trace-cmd: Replace PySting_FromString() with PyUnicode_FromString()
      trace-cmd: Use PyMemoryView_FromMemory() for Python 3
      trace-cmd: Use PyLong_AsLong() for Python 3

----
 python/ctracecmd.i | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

[PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString()

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Python 3 has deprecated PyString_FromString(), but both Python 2 and
Python 3 have PyUnicode_FromString() which should be equivalent (at least
according to google). As Python 2 is going to be EOL soon, we need to
support Python 3.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204231

Reported-by: Troy Engel <troyengel@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 python/ctracecmd.i | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/python/ctracecmd.i b/python/ctracecmd.i
index 65a3d5144b88..09d1d6414fc1 100644
--- a/python/ctracecmd.i
+++ b/python/ctracecmd.i
@@ -108,7 +108,7 @@ static PyObject *py_field_get_stack(struct tep_handle *pevent,
 		    ((int)addr == -1))
 			break;
 		func = tep_find_function(event->tep, addr);
-		if (PyList_Append(list, PyString_FromString(func))) {
+		if (PyList_Append(list, PyUnicode_FromString(func))) {
 			Py_DECREF(list);
 			return NULL;
 		}
@@ -162,10 +162,10 @@ static PyObject *py_field_get_str(struct tep_format_field *f, struct tep_record
 		 */
 		offset = val & 0xffff;
 
-		return PyString_FromString((char *)r->data + offset);
+		return PyUnicode_FromString((char *)r->data + offset);
 	}
 
-	return PyString_FromStringAndSize((char *)r->data + f->offset,
+	return PyUnicode_FromStringAndSize((char *)r->data + f->offset,
 				strnlen((char *)r->data + f->offset, f->size));
 }
 
@@ -177,7 +177,7 @@ static PyObject *py_format_get_keys(struct tep_event *ef)
 	list = PyList_New(0);
 
 	for (f = ef->format.fields; f; f = f->next) {
-		if (PyList_Append(list, PyString_FromString(f->name))) {
+		if (PyList_Append(list, PyUnicode_FromString(f->name))) {
 			Py_DECREF(list);
 			return NULL;
 		}
-- 
2.20.1

[PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Python 3 has deprecated PyBuffer_FromMemory() and instead has
PyMemoryView_FromMemory(). Add a helper function that uses the latter if
Python 3 is detected. As Python 2 is going to be EOL soon, we need to
support Python 3.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204231

Reported-by: Troy Engel <troyengel@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 python/ctracecmd.i | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/python/ctracecmd.i b/python/ctracecmd.i
index 09d1d6414fc1..63e5dcb813f1 100644
--- a/python/ctracecmd.i
+++ b/python/ctracecmd.i
@@ -117,6 +117,15 @@ static PyObject *py_field_get_stack(struct tep_handle *pevent,
 	return list;
 }
 
+static PyObject *fromMemory(void *buf, size_t len)
+{
+#if PY_MAJOR_VERSION >= 3
+		return PyMemoryView_FromMemory(buf, len, PyBUF_READ);
+#else
+		return PyBuffer_FromMemory(buf, len);
+#endif
+}
+
 static PyObject *py_field_get_data(struct tep_format_field *f, struct tep_record *r)
 {
 	if (!strncmp(f->type, "__data_loc ", 11)) {
@@ -137,10 +146,10 @@ static PyObject *py_field_get_data(struct tep_format_field *f, struct tep_record
 		offset = val & 0xffff;
 		len = val >> 16;
 
-		return PyBuffer_FromMemory((char *)r->data + offset, len);
+		return fromMemory(r->data + offset, len);
 	}
 
-	return PyBuffer_FromMemory((char *)r->data + f->offset, f->size);
+	return fromMemory(r->data + f->offset, f->size);
 }
 
 static PyObject *py_field_get_str(struct tep_format_field *f, struct tep_record *r)
-- 
2.20.1

[PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Python 3 has deprecated PyInt_AS_LONG. Add code to use PyLong_AsLong() if
Python 3 is detected. As Python 2 is going to be EOL soon, we need to
support Python 3.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204231

Reported-by: Troy Engel <troyengel@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 python/ctracecmd.i | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/python/ctracecmd.i b/python/ctracecmd.i
index 63e5dcb813f1..2601d39a76be 100644
--- a/python/ctracecmd.i
+++ b/python/ctracecmd.i
@@ -117,14 +117,21 @@ static PyObject *py_field_get_stack(struct tep_handle *pevent,
 	return list;
 }
 
+#if PY_MAJOR_VERSION >= 3
 static PyObject *fromMemory(void *buf, size_t len)
 {
-#if PY_MAJOR_VERSION >= 3
 		return PyMemoryView_FromMemory(buf, len, PyBUF_READ);
+}
+#define PY_INT_AS_LONG PyLong_AsLong
 #else
+static PyObject *fromMemory(void *buf, size_t len)
+{
 		return PyBuffer_FromMemory(buf, len);
-#endif
 }
+#define PY_INT_AS_LONG PyInt_AS_LONG
+#endif
+
+
 
 static PyObject *py_field_get_data(struct tep_format_field *f, struct tep_record *r)
 {
@@ -226,7 +233,7 @@ static int python_callback(struct trace_seq *s,
 			Py_XDECREF(result);
 			return 0;
 		}
-		r = PyInt_AS_LONG(result);
+		r = PY_INT_AS_LONG(result);
 	} else if (result == Py_None)
 		r = 0;
 	else
-- 
2.20.1

Re: [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString()

[Johannes Berg]

On Fri, 2019-07-19 at 18:46 -0400, Steven Rostedt wrote:

> --- a/python/ctracecmd.i
> +++ b/python/ctracecmd.i
> @@ -108,7 +108,7 @@ static PyObject *py_field_get_stack(struct tep_handle *pevent,
>  		    ((int)addr == -1))
>  			break;
>  		func = tep_find_function(event->tep, addr);
> -		if (PyList_Append(list, PyString_FromString(func))) {
> +		if (PyList_Append(list, PyUnicode_FromString(func))) {
>  			Py_DECREF(list);

This assumes that the source code is not using extended identifiers (per
C99, gcc -fextended-identifiers). I think that's probably a reasonable
assumption :-)

In theory, a bit safer would be to use PyBytes_FromString() and let the
python plugin worry about the encoding.

That really applies to all of the instances here, I think, since the
strings eventually come from (macros in the) code.

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes

Re: [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3

[Johannes Berg]

On Fri, 2019-07-19 at 18:46 -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> Python 3 has deprecated PyBuffer_FromMemory() and instead has
> PyMemoryView_FromMemory(). Add a helper function that uses the latter if
> Python 3 is detected. As Python 2 is going to be EOL soon, we need to
> support Python 3.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204231

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes

Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3

[Johannes Berg]

On Fri, 2019-07-19 at 18:46 -0400, Steven Rostedt wrote:
 
> +#if PY_MAJOR_VERSION >= 3
[...]
> +#define PY_INT_AS_LONG PyLong_AsLong
>  #else
[...]
> +#define PY_INT_AS_LONG PyInt_AS_LONG
> +#endif

I probably would've gone for only having a single

 #define PyLong_AsLong PyInt_AS_LONG

in the python2 case, but doesn't really matter.


Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes

Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3

[Steven Rostedt]

On Fri, 26 Jul 2019 09:31:57 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:

> On Fri, 2019-07-19 at 18:46 -0400, Steven Rostedt wrote:
>  
> > +#if PY_MAJOR_VERSION >= 3  
> [...]
> > +#define PY_INT_AS_LONG PyLong_AsLong
> >  #else  
> [...]
> > +#define PY_INT_AS_LONG PyInt_AS_LONG
> > +#endif  
> 
> I probably would've gone for only having a single
> 
>  #define PyLong_AsLong PyInt_AS_LONG
> 
> in the python2 case, but doesn't really matter.
> 
> 
> Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
> 

Hi Johannes,

Thanks for the review (unfortunately, due to the rush to get
KernelShark 1.0 out, I already applied them, I should have Cc'd you on
them too).

Should we make any of the above changes you mentioned to any of these
patches? I'm looking at releasing trace-cmd 2.8.4 with all these
changes and will hold off if you think it would be better to update
them.

Thanks!

-- Steve

Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3

[Johannes Berg]

Hi,

> Thanks for the review (unfortunately, due to the rush to get
> KernelShark 1.0 out, I already applied them, I should have Cc'd you on
> them too).

> 
> Should we make any of the above changes you mentioned to any of these
> patches? I'm looking at releasing trace-cmd 2.8.4 with all these
> changes and will hold off if you think it would be better to update
> them.

I don't think it matters.

The PyLong thing was just really cosmetic, could've saved some lines of
code.

The bytes vs. unicode - well, python assumes that you give it a utf-8
string, so even if we start using non-ASCII identifiers (which I think
is highly unlikely!), we'll likely use utf-8 in the kernel, and it would
either way not be an issue.

johannes

Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3

[Steven Rostedt]

On Fri, 26 Jul 2019 17:22:58 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:


> The bytes vs. unicode - well, python assumes that you give it a utf-8
> string, so even if we start using non-ASCII identifiers (which I think
> is highly unlikely!), we'll likely use utf-8 in the kernel, and it would
> either way not be an issue.

Thanks Johannes for explaining.

-- Steve