This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
>>>>> "Paul" == <Paul_Koning@Dell.com> writes:
Paul> Here is an updated patch which I believe addresses all the comments.
Thanks.
Paul> Should I add a NEWS entry for the Python 3 support?
Yes, that would be good.
Paul> PyTypeObject breakpoint_object_type =
Paul> {
Paul> - PyObject_HEAD_INIT (NULL)
Paul> - 0, /*ob_size*/
Paul> + PyVarObject_HEAD_INIT (NULL, 0)
The new line has the wrong indentation.
Paul> static PyTypeObject cmdpy_object_type =
Paul> {
Paul> - PyObject_HEAD_INIT (NULL)
Paul> - 0, /*ob_size*/
Paul> + PyVarObject_HEAD_INIT (NULL, 0)
Ditto.
Paul> PyTypeObject event_object_type =
Paul> {
Paul> - PyObject_HEAD_INIT (NULL)
Paul> - 0, /* ob_size */
Paul> + PyVarObject_HEAD_INIT (NULL, 0)
Ditto.
Paul> static PyTypeObject eventregistry_object_type =
Paul> {
Paul> - PyObject_HEAD_INIT (NULL)
Paul> - 0, /* ob_size */
Paul> + PyVarObject_HEAD_INIT (NULL, 0)
Ditto.
Paul> + if (except.reason < 0)
Paul> + PyBuffer_Release (&pybuf);
Paul> GDB_PY_HANDLE_EXCEPTION (except);
Paul> +#ifdef IS_PY3K
Paul> + PyBuffer_Release (&pybuf);
Paul> +#endif
I think the first PyBuffer_Release also has to be conditional on IS_PY3K
here.
It seems like you could just always call it after the try-catch rather
than checking except.reason.
Paul> +#ifdef IS_PY3K
Paul> +static int
Blank line between these two.
Paul> @@ -638,6 +698,10 @@
Paul> }
Paul> GDB_PY_HANDLE_EXCEPTION (except);
Paul> +#ifdef IS_PY3K
Paul> + PyBuffer_Release (&pybuf);
Paul> +#endif
It seems like PyBuffer_Release must be called before
GDB_PY_HANDLE_EXCEPTION here.
Paul> +#ifdef IS_PY3K
Paul> +static PyBufferProcs buffer_procs = {
Blank line between these two, and I think "{" on its own line is
generally preferred.
Paul> +++ gdb/python/py-value.c 14 Nov 2012 15:57:17 -0000
Paul> @@ -106,7 +106,7 @@
Paul> Py_XDECREF (self->dynamic_type);
Paul> - self->ob_type->tp_free (self);
Paul> + obj->ob_type->tp_free (self);
Paul> }
Doesn't this need the new Py_TYPE macro?
Paul> +/* FIXME: there are a lot of calls below that do not check the return
Paul> + value for errors. */
Don't bother with the new FIXME.
Paul> + int python_init_ok = 0;
This seems to be unused.
Paul> + progname_copy = PyMem_Malloc ((progsize + 1) * sizeof (wchar_t));
[...]
Paul> + Py_SetProgramName (progname_copy);
I think a comment here explaining why we never free progname_copy would
be nice to have.
thanks,
Tom
- References:
- [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)
- Re: [PATCH] Python 3 support, part 1 (non-testsuite part)