Re: [PATCH] Fix Python crash on getprop deallocation

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On Fri, Dec 24, 2021 at 06:17:25AM -0700, Simon Glass wrote:
> Hi Luca,
> 
> On Fri, 24 Dec 2021 at 03:38, Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org> wrote:
> >
> > Fatal Python error: none_dealloc: deallocating None
> > Python runtime state: finalizing (tstate=0x000055c9bac70920)
> >
> > Current thread 0x00007fbe34e47740 (most recent call first):
> >   <no Python frame>
> > Aborted (core dumped)
> >
> > This is caused by a missing Py_INCREF on the returned Py_None, as
> > demonstrated e.g. in https://github.com/mythosil/swig-python-incref or
> > described at https://edcjones.tripod.com/refcount.html ("Remember to
> > INCREF Py_None!")
> >
> > A PoC for triggering this crash is uploaded to
> > https://github.com/z3ntu/pylibfdt-crash .
> > With this patch applied to pylibfdt the crash does not happen.
> >
> > Signed-off-by: Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org>
> > ---
> > Unrelated but I've noticed that in this file the indentation is quite
> > mixed between spaces and tabs. This patch tries to keep to the style in
> > the lines around.
> >
> >  pylibfdt/libfdt.i | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> My original idea was to use tabs for the C code (to match the libfdt
> style) and spaces for the Python code (for PEP8). Looking at it now,
> that idea has not continued and I'm not even sure it was a good idea.

Sounds like a good rationale, but probably not practical when combined
into a single file.  I'd suggest making it all spaces.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]