Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] trace-cmd: Update python plugin for Python 3
@ 2019-07-19 22:46 Steven Rostedt
  2019-07-19 22:46 ` [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString() Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Steven Rostedt @ 2019-07-19 22:46 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Johannes Berg, Josef Bacik, Darren Hart, troyengel

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(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString()
  2019-07-19 22:46 [PATCH 0/3] trace-cmd: Update python plugin for Python 3 Steven Rostedt
@ 2019-07-19 22:46 ` Steven Rostedt
  2019-07-26  7:29   ` Johannes Berg
  2019-07-19 22:46 ` [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3 Steven Rostedt
  2019-07-19 22:46 ` [PATCH 3/3] trace-cmd: Use PyLong_AsLong() " Steven Rostedt
  2 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2019-07-19 22:46 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Johannes Berg, Josef Bacik, Darren Hart, troyengel

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3
  2019-07-19 22:46 [PATCH 0/3] trace-cmd: Update python plugin for Python 3 Steven Rostedt
  2019-07-19 22:46 ` [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString() Steven Rostedt
@ 2019-07-19 22:46 ` Steven Rostedt
  2019-07-26  7:30   ` Johannes Berg
  2019-07-19 22:46 ` [PATCH 3/3] trace-cmd: Use PyLong_AsLong() " Steven Rostedt
  2 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2019-07-19 22:46 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Johannes Berg, Josef Bacik, Darren Hart, troyengel

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3
  2019-07-19 22:46 [PATCH 0/3] trace-cmd: Update python plugin for Python 3 Steven Rostedt
  2019-07-19 22:46 ` [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString() Steven Rostedt
  2019-07-19 22:46 ` [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3 Steven Rostedt
@ 2019-07-19 22:46 ` " Steven Rostedt
  2019-07-26  7:31   ` Johannes Berg
  2 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2019-07-19 22:46 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Johannes Berg, Josef Bacik, Darren Hart, troyengel

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString()
  2019-07-19 22:46 ` [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString() Steven Rostedt
@ 2019-07-26  7:29   ` Johannes Berg
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Berg @ 2019-07-26  7:29 UTC (permalink / raw)
  To: Steven Rostedt, linux-trace-devel; +Cc: Josef Bacik, Darren Hart, troyengel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3
  2019-07-19 22:46 ` [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3 Steven Rostedt
@ 2019-07-26  7:30   ` Johannes Berg
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Berg @ 2019-07-26  7:30 UTC (permalink / raw)
  To: Steven Rostedt, linux-trace-devel; +Cc: Josef Bacik, Darren Hart, troyengel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3
  2019-07-19 22:46 ` [PATCH 3/3] trace-cmd: Use PyLong_AsLong() " Steven Rostedt
@ 2019-07-26  7:31   ` Johannes Berg
  2019-07-26 15:18     ` Steven Rostedt
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2019-07-26  7:31 UTC (permalink / raw)
  To: Steven Rostedt, linux-trace-devel; +Cc: Josef Bacik, Darren Hart, troyengel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3
  2019-07-26  7:31   ` Johannes Berg
@ 2019-07-26 15:18     ` Steven Rostedt
  2019-07-26 15:22       ` Johannes Berg
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2019-07-26 15:18 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-trace-devel, Josef Bacik, Darren Hart, troyengel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3
  2019-07-26 15:18     ` Steven Rostedt
@ 2019-07-26 15:22       ` Johannes Berg
  2019-07-26 15:30         ` Steven Rostedt
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2019-07-26 15:22 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel, Josef Bacik, Darren Hart, troyengel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] trace-cmd: Use PyLong_AsLong() for Python 3
  2019-07-26 15:22       ` Johannes Berg
@ 2019-07-26 15:30         ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2019-07-26 15:30 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-trace-devel, Josef Bacik, Darren Hart, troyengel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-19 22:46 [PATCH 0/3] trace-cmd: Update python plugin for Python 3 Steven Rostedt
2019-07-19 22:46 ` [PATCH 1/3] trace-cmd: Replace PySting_FromString() with PyUnicode_FromString() Steven Rostedt
2019-07-26  7:29   ` Johannes Berg
2019-07-19 22:46 ` [PATCH 2/3] trace-cmd: Use PyMemoryView_FromMemory() for Python 3 Steven Rostedt
2019-07-26  7:30   ` Johannes Berg
2019-07-19 22:46 ` [PATCH 3/3] trace-cmd: Use PyLong_AsLong() " Steven Rostedt
2019-07-26  7:31   ` Johannes Berg
2019-07-26 15:18     ` Steven Rostedt
2019-07-26 15:22       ` Johannes Berg
2019-07-26 15:30         ` Steven Rostedt

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org linux-trace-devel@archiver.kernel.org
	public-inbox-index linux-trace-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox