Re: [PATCH 1/1] tools, xen/scripts: clear out Python syntax warnings

["Ariel Otilibili-Anieli" <Ariel.Otilibili-Anieli@xxxxxxxxxx>]

On Monday, December 16, 2024 12:34 CET, Andrew Cooper 
<andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Cc: Luca Fancellu <luca.fancellu@xxxxxxx>
> > Signed-off-by: Ariel Otilibili <Ariel.Otilibili-Anieli@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx>

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