Re: [PATCH 1/2] docs: python: abi_regex: catch the right exception for a bad regex

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch replaces `re.PatternError` with `re.error` rather than catching both, directly contradicting the commit message and exposing a misunderstanding of standard Python.
--

--- Patch [1]: [PATCH 1/2] docs: python: abi_regex: catch the right exception for a bad regex ---
commit ed601931671f151d8ef4f1bbd0e817520a34f3c5
Author: Alison Schofield <alison.schofield@intel.com>

docs: python: abi_regex: catch the right exception for a bad regex

This commit updates the exception handling in abi_regex.py to prevent the
script from aborting on invalid regular expressions. It aims to support
both older and newer Python releases by handling regex errors correctly.

> Older Python releases raise re.error, while newer releases expose
> re.PatternError. Catching only the newer name causes the scan to
> abort when an invalid expression is encountered.
> 
> Catch both names so the scan continues and reports the remaining
> results.

[Severity: Medium]
Does the commit message accurately reflect the changes? 

The text states it will "Catch both names", but the patch replaces 
re.PatternError with re.error entirely. While standard Python uses re.error, 
if newer releases actually exposed re.PatternError as stated in the commit 
message, wouldn't replacing the handler cause newer Python releases to abort?

> diff --git a/tools/lib/python/abi/abi_regex.py b/tools/lib/python/abi/abi_regex.py
> --- a/tools/lib/python/abi/abi_regex.py
> +++ b/tools/lib/python/abi/abi_regex.py
> @@ -155,7 +155,7 @@ class AbiRegex(AbiParser):
>              if self.search_string:
>                  if what.find(self.search_string) >= 0:
>                      print(f"What: {what}")
> -        except re.PatternError:
> +        except re.error:

[Severity: Medium]
Is it intentional to only catch re.error here in regex_append(), rather
than catching both as described in the commit message?

[ ... ]

> @@ -194,7 +194,7 @@ class AbiRegex(AbiParser):
>  
>                  try:
>                      self.re_string = re.compile(self.search_string)
> -                except re.PatternError as e:
> +                except re.error as e:

[Severity: Medium]
Does this __init__() block also need to catch both exceptions, or is
replacing re.PatternError with re.error sufficient?

[ ... ]

> @@ -223,9 +223,9 @@ class AbiRegex(AbiParser):
>                  for r, s in self.re_whats:
>                      try:
>                          new = r.sub(s, new)
> -                    except re.PatternError as e:
> +                    except re.error as e:
>                          # Help debugging troubles with new regexes
> -                        raise re.PatternError(f"{e}\nwhile re.sub('{r.pattern}', {s}, str)") from e
> +                        raise re.error(f"{e}\nwhile re.sub('{r.pattern}', {s}, str)") from e

[Severity: Medium]
Similar to the other locations, parse_abi() replaces the exception class
instead of catching both. Does this match the intended behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/d57d3ddc0d7d883bd61ad07c6d3969b622fd446f.1786058698.git.alison.schofield@intel.com?part=1