All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ariel Otilibili-Anieli" <Ariel.Otilibili-Anieli@eurecom.fr>
To: "Andrew Cooper" <andrew.cooper3@citrix.com>
Cc: xen-devel@lists.xenproject.org, "Jan Beulich" <jbeulich@suse.com>,
	"Luca Fancellu" <luca.fancellu@arm.com>,
	"Anthony PERARD" <anthony.perard@vates.tech>
Subject: Re: [PATCH 1/1] tools, xen/scripts: clear out Python syntax warnings
Date: Mon, 16 Dec 2024 17:51:39 +0100	[thread overview]
Message-ID: <2f7a87-67605a80-ce37-141f3260@214819454> (raw)
In-Reply-To: <49497f8c-a2e4-49a1-aac0-96d704834f0f@citrix.com>

On Monday, December 16, 2024 12:34 CET, Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> On 14/12/2024 4:09 pm, Ariel Otilibili wrote:
> > * since 3.12 invalid escape sequences generate SyntaxWarning
> > * in the future, these invalid sequences will generate SyntaxError
> > * therefore changed syntax to raw string notation.
> >
> > Link: https://docs.python.org/3/whatsnew/3.12.html#other-language-changes
> > Fixes: e45e8f69047 ("bitkeeper revision 1.803 (4056f51d2UjBnn9uwzC9Vu3LspnUCg)")
> > Fixes: d8f3a67bf98 ("pygrub: further improve grub2 support")
> > Fixes: dd03048708a ("xen/pygrub: grub2/grub.cfg from RHEL 7 has new commands in menuentry")
> > Fixes: d1b93ea2615 ("tools/pygrub: Make pygrub understand default entry in string format")
> > Fixes: 622e368758b ("Add ZFS libfsimage support patch")
> > Fixes: 02b26c02c7c ("xen/scripts: add cppcheck tool to the xen-analysis.py script")
> > Fixes: 56c0063f4e7 ("xen/misra: xen-analysis.py: Improve the cppcheck version check")
> >
> > Cc: Anthony PERARD <anthony.perard@vates.tech>
> > Cc: Luca Fancellu <luca.fancellu@arm.com>
> > Signed-off-by: Ariel Otilibili <Ariel.Otilibili-Anieli@eurecom.fr>
> > ---
> >  tools/misc/xensymoops                         | 4 ++--
> >  tools/pygrub/src/GrubConf.py                  | 4 ++--
> >  tools/pygrub/src/pygrub                       | 6 +++---
> >  xen/scripts/xen_analysis/cppcheck_analysis.py | 4 ++--
> >  4 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/misc/xensymoops b/tools/misc/xensymoops
> > index 835d187e90..bec75cae93 100755
> > --- a/tools/misc/xensymoops
> > +++ b/tools/misc/xensymoops
> > @@ -17,7 +17,7 @@ def read_oops():
> >      stack_addrs is a dictionary mapping potential code addresses in the stack
> >        to their order in the stack trace.
> >      """
> > -    stackaddr_ptn = "\[([a-z,0-9]*)\]"
> > +    stackaddr_ptn = r"\[([a-z,0-9]*)\]"
> >      stackaddr_re  = re.compile(stackaddr_ptn)
> >  
> >      eip_ptn = ".*EIP:.*<([a-z,0-9]*)>.*"
> 
> Oh wow.  I've not come across this script before, and it's not
> referenced in the build system.
> 
> Also, it's hard-coded to 32bit Xen which was deleted in Xen 4.13 more
> than a decade ago, and there are other errors in the regexes such as
> including a comma in stackaddr_ptn
> 
> Worse however, it escaped the Py2->3 conversion and is still using raw
> print statements.
> 
> I'll submit a patch deleting it entirely.

Acked-by: Ariel Otilibili-Anieli <Ariel.Otilibili-Anieli@eurecom.fr>

I'll send a new series, only on the subsequent feedback. 
> 
> > diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
> > index 580c9628ca..7cd2bc9aeb 100644
> > --- a/tools/pygrub/src/GrubConf.py
> > +++ b/tools/pygrub/src/GrubConf.py
> > @@ -320,7 +320,7 @@ class GrubConfigFile(_GrubConfigFile):
> >  def grub2_handle_set(arg):
> >      (com,arg) = grub_split(arg,2)
> >      com="set:" + com
> > -    m = re.match("([\"\'])(.*)\\1", arg)
> > +    m = re.match(r"([\"\'])(.*)\\1", arg)
> 
> Doesn't this \\1 want to turn into just \1 now it's a raw string?

Indeed; I'll do that.
> 
> > diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> > index 9d51f96070..58b088d285 100755
> > --- a/tools/pygrub/src/pygrub
> > +++ b/tools/pygrub/src/pygrub
> > @@ -1104,7 +1104,7 @@ if __name__ == "__main__":
> >      if chosencfg["args"]:
> >          zfsinfo = xenfsimage.getbootstring(fs)
> >          if zfsinfo is not None:
> > -            e = re.compile("zfs-bootfs=[\w\-\.\:@/]+" )
> > +            e = re.compile(r"zfs-bootfs=[\w\-\.\:@/]+" )
> 
> Related, this string looks dodgy.  The \- is correct (I think, to not
> have it interpreted as a range), but I'm pretty sure a literal . and :
> don't need escaping inside a [], and the result here would be for a
> literal \ to be included.
> 

To replace: \w\-\.\:@/
By: \w\-.:@/

Is this what you mean?
> ~Andrew



  reply	other threads:[~2024-12-16 16:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-14 16:09 [PATCH 0/1] tools, xen/scripts: clear out Python syntax warnings Ariel Otilibili
2024-12-14 16:09 ` [PATCH 1/1] " Ariel Otilibili
2024-12-16 11:34   ` Andrew Cooper
2024-12-16 16:51     ` Ariel Otilibili-Anieli [this message]
2024-12-16 23:07 ` [PATCH v2 0/1] " Ariel Otilibili
2024-12-16 23:07   ` [PATCH v2 1/1] " Ariel Otilibili
2024-12-17  8:31     ` Luca Fancellu
2024-12-17 13:26       ` Ariel Otilibili-Anieli
2024-12-17 16:26     ` Andrew Cooper
2024-12-17 17:13       ` Ariel Otilibili-Anieli
2024-12-18 14:21         ` Andrew Cooper
2024-12-18 15:20           ` Ariel Otilibili-Anieli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2f7a87-67605a80-ce37-141f3260@214819454 \
    --to=ariel.otilibili-anieli@eurecom.fr \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=jbeulich@suse.com \
    --cc=luca.fancellu@arm.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.