Re: [PATCH 1/5] gdb/python: handle saving user registers in a frame unwinder

Lancelot SIX via Gdb-patches
2021-06-07T18:02:47Z

On Mon, Jun 07, 2021 at 01:20:33PM -0400, Simon Marchi wrote:
> On 2021-06-07 1:07 p.m., Lancelot SIX via Gdb-patches wrote:
> > Hi,
> >
> > I just have a minor stylistic remark in the python code in the test:
> >
> >> […]
> >> +
> >> + def __call__(self, pending_frame):
> >> + pc_desc = pending_frame.architecture().registers().find("pc")
> >> + pc = pending_frame.read_register(pc_desc)
> >> +
> >> + sp_desc = pending_frame.architecture().registers().find("sp")
> >> + sp = pending_frame.read_register(sp_desc)
> >> +
> >> + block = gdb.block_for_pc(int(pc))
> >> + if block == None:
> >
> > When looking for None, it is usually prefered to use 'is None' instead
> > of '== None'. The result is the same unless there is a strange overload
> > of __eq__.
> >
> > This pattern can also be seen in patch 3 and 4 of your series (patch 4
> > using both '==' and 'is' to check for None).
>
> I agree, that's the convention in Python. It is not in our coding
> standards, but I suggest using flake8 to check the Python code, it
> reports this (and much more):

Hi,

Actually, this is mentioned in the PEP-8[1][2], which states in the
“Programming Recommandations” section:

Comparisons to singletons like None should always be done with is or
is not, never the equality operators.

This leads me to an annex question. Given that I still lack a lot of
experience with the overall codebase, I tend to pick this kind of small
stylistic details more easily than design and logic problems. I do not
always point out those I see when I read the ML, but I can totally
understand those isolated stylistic comments can be considered as noise.
If so, please let me know!

>
>
> $ flake8 testsuite/gdb.python/py-unwind-user-regs.py
> testsuite/gdb.python/py-unwind-user-regs.py:52:18: E711 comparison to None should be 'if cond is None:'
> testsuite/gdb.python/py-unwind-user-regs.py:55:17: E711 comparison to None should be 'if cond is None:'

I am currently running the testsuite against a patch that fixes those I
found. I’ll try to post it later tonight.

Lancelot.

>
> Simon

[1] https://www.python.org/dev/peps/pep-0008/#programming-recommendations
[2] https://sourceware.org/gdb/wiki/Internals%20GDB-Python-Coding-Standards

--
Lancelot SIX