| Message ID | 20230608135913.560413-2-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] tools: Fix ifdef for aarch64 that should include also arm | expand |
> On 8 Jun 2023, at 14:59, Luca Fancellu <Luca.Fancellu@arm.com> wrote: > > Commit 56a7aaa16bfe introduced a memory leak on the error path for a > Py_BuildValue built object that on some newly introduced error path > has not the correct reference count handling, fix that by decrementing > the refcount in these path. > > Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm") > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- Hi all, Is there any chance to have this one reviewed by the end of the month? I’m asking because I have a Jira task attached to this patch and my PM is chasing me :) If it’s not possible it’s fine either and I’ll have just to report that. Cheers, Luca > tools/python/xen/lowlevel/xc/xc.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c > index e14e223ec903..d3ea350e07b9 100644 > --- a/tools/python/xen/lowlevel/xc/xc.c > +++ b/tools/python/xen/lowlevel/xc/xc.c > @@ -919,11 +919,16 @@ static PyObject *pyxc_physinfo(XcObject *self) > sve_vl_bits = arch_capabilities_arm_sve(pinfo.arch_capabilities); > py_arm_sve_vl = PyLong_FromUnsignedLong(sve_vl_bits); > > - if ( !py_arm_sve_vl ) > + if ( !py_arm_sve_vl ) { > + Py_DECREF(objret); > return NULL; > + } > > - if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) ) > + if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) ) { > + Py_DECREF(py_arm_sve_vl); > + Py_DECREF(objret); > return NULL; > + } > } > #endif > > -- > 2.34.1 > >
On Thu, Jun 08, 2023 at 02:59:13PM +0100, Luca Fancellu wrote: > Commit 56a7aaa16bfe introduced a memory leak on the error path for a > Py_BuildValue built object that on some newly introduced error path > has not the correct reference count handling, fix that by decrementing > the refcount in these path. > > Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm") > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > tools/python/xen/lowlevel/xc/xc.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c > index e14e223ec903..d3ea350e07b9 100644 > --- a/tools/python/xen/lowlevel/xc/xc.c > +++ b/tools/python/xen/lowlevel/xc/xc.c > @@ -919,11 +919,16 @@ static PyObject *pyxc_physinfo(XcObject *self) > sve_vl_bits = arch_capabilities_arm_sve(pinfo.arch_capabilities); > py_arm_sve_vl = PyLong_FromUnsignedLong(sve_vl_bits); > > - if ( !py_arm_sve_vl ) > + if ( !py_arm_sve_vl ) { > + Py_DECREF(objret); > return NULL; > + } > > - if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) ) > + if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) ) { > + Py_DECREF(py_arm_sve_vl); > + Py_DECREF(objret); > return NULL; > + } > } > #endif > > -- > 2.34.1 >
On Mon, Jun 26, 2023 at 08:07:46PM +0200, Marek Marczykowski-Górecki wrote: > On Thu, Jun 08, 2023 at 02:59:13PM +0100, Luca Fancellu wrote: > > Commit 56a7aaa16bfe introduced a memory leak on the error path for a > > Py_BuildValue built object that on some newly introduced error path > > has not the correct reference count handling, fix that by decrementing > > the refcount in these path. > > > > Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm") > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Oh, and BTW, in relation to the discussion on the summit about committing process, the buggy version was committed without my ack, after waiting for my review for about two weeks.
Hi Marek, On 27/06/2023 13:29, Marek Marczykowski-Górecki wrote: > On Mon, Jun 26, 2023 at 08:07:46PM +0200, Marek Marczykowski-Górecki wrote: >> On Thu, Jun 08, 2023 at 02:59:13PM +0100, Luca Fancellu wrote: >>> Commit 56a7aaa16bfe introduced a memory leak on the error path for a >>> Py_BuildValue built object that on some newly introduced error path >>> has not the correct reference count handling, fix that by decrementing >>> the refcount in these path. >>> >>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm") >>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >> >> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > Oh, and BTW, in relation to the discussion on the summit about > committing process, the buggy version was committed without my ack, > after waiting for my review for about two weeks. I was the committer for the series. In this case, this was not a case of committing because I thought the patch was uncontroversial. Instead, this was a lack of my side to properly check the acks on the patch. Sorry for that. In general, as I mentioned during the design session, I don't commit with missing acks without explicitly saying so on the mailing list and this is often after consulting with others on IRC. On a similar topic. So far, checking the ack is a manual process for me. So this is not entirely an error-free process (in particular for patch requiring multiple maintainers acks). I would love to have a script that could tell me if a patch is fully approved and/or what acks are missing. Cheers,
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c index e14e223ec903..d3ea350e07b9 100644 --- a/tools/python/xen/lowlevel/xc/xc.c +++ b/tools/python/xen/lowlevel/xc/xc.c @@ -919,11 +919,16 @@ static PyObject *pyxc_physinfo(XcObject *self) sve_vl_bits = arch_capabilities_arm_sve(pinfo.arch_capabilities); py_arm_sve_vl = PyLong_FromUnsignedLong(sve_vl_bits); - if ( !py_arm_sve_vl ) + if ( !py_arm_sve_vl ) { + Py_DECREF(objret); return NULL; + } - if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) ) + if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) ) { + Py_DECREF(py_arm_sve_vl); + Py_DECREF(objret); return NULL; + } } #endif
Commit 56a7aaa16bfe introduced a memory leak on the error path for a Py_BuildValue built object that on some newly introduced error path has not the correct reference count handling, fix that by decrementing the refcount in these path. Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm") Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> --- tools/python/xen/lowlevel/xc/xc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)