From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id 9BA573896823 for ; Mon, 7 Jun 2021 18:01:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9BA573896823 Received: from Plymouth (unknown [IPv6:2a02:390:9086:0:7bc6:28c0:e505:21fc]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 0763281895; Mon, 7 Jun 2021 18:01:34 +0000 (UTC) Date: Mon, 7 Jun 2021 19:01:31 +0100 From: Lancelot SIX To: Simon Marchi Cc: Andrew Burgess , gdb-patches@sourceware.org Subject: Re: [PATCH 1/5] gdb/python: handle saving user registers in a frame unwinder Message-ID: <20210607180131.dtq5ejhffpigicue@Plymouth> References: <20210607170710.a2dtukyyjjxwzmes@Plymouth> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Mon, 07 Jun 2021 18:01:35 +0000 (UTC) X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Jun 2021 18:01:47 -0000 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