[PATCH] gdb: copyright: make file header scan a bit more pythonic

[PATCH] gdb: copyright: make file header scan a bit more pythonic

[Mike Frysinger]

Should be functionally the same, but uses more pythonic idioms to get
fewer lines of code, and to make sure to not leak open file handles.
---
 gdb/copyright.py | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/gdb/copyright.py b/gdb/copyright.py
index a78f7f2aa9b0..918d2e473d49 100755
--- a/gdb/copyright.py
+++ b/gdb/copyright.py
@@ -148,15 +148,12 @@ def may_have_copyright_notice(filename):
     # so just open the file as a byte stream. We only need to search
     # for a pattern that should be the same regardless of encoding,
     # so that should be good enough.
-    fd = open(filename, "rb")
-
-    lineno = 1
-    for line in fd:
-        if b"Copyright" in line:
-            return True
-        lineno += 1
-        if lineno > 50:
-            return False
+    with open(filename, "rb") as fd:
+        for lineno, line in enumerate(fd, start=1):
+            if b"Copyright" in line:
+                return True
+            if lineno > 50:
+                break
     return False
 
 
-- 
2.33.0

Re: [PATCH] gdb: copyright: make file header scan a bit more pythonic

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

ping ...
-mike

On 01 Jan 2022 13:15, Mike Frysinger via Gdb-patches wrote:
> Should be functionally the same, but uses more pythonic idioms to get
> fewer lines of code, and to make sure to not leak open file handles.
> ---
>  gdb/copyright.py | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/copyright.py b/gdb/copyright.py
> index a78f7f2aa9b0..918d2e473d49 100755
> --- a/gdb/copyright.py
> +++ b/gdb/copyright.py
> @@ -148,15 +148,12 @@ def may_have_copyright_notice(filename):
>      # so just open the file as a byte stream. We only need to search
>      # for a pattern that should be the same regardless of encoding,
>      # so that should be good enough.
> -    fd = open(filename, "rb")
> -
> -    lineno = 1
> -    for line in fd:
> -        if b"Copyright" in line:
> -            return True
> -        lineno += 1
> -        if lineno > 50:
> -            return False
> +    with open(filename, "rb") as fd:
> +        for lineno, line in enumerate(fd, start=1):
> +            if b"Copyright" in line:
> +                return True
> +            if lineno > 50:
> +                break
>      return False
>  
>  
> -- 
> 2.33.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gdb: copyright: make file header scan a bit more pythonic

[Joel Brobecker]

Hi Mike,

On Mon, Oct 24, 2022 at 01:07:17AM +0545, Mike Frysinger via Gdb-patches wrote:
> ping ...
> -mike
> 
> On 01 Jan 2022 13:15, Mike Frysinger via Gdb-patches wrote:
> > Should be functionally the same, but uses more pythonic idioms to get
> > fewer lines of code, and to make sure to not leak open file handles.

Sorry about the delay in reviewing. The patch is approved and you can
push it.

Thanks for the patch.

> > ---
> >  gdb/copyright.py | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/gdb/copyright.py b/gdb/copyright.py
> > index a78f7f2aa9b0..918d2e473d49 100755
> > --- a/gdb/copyright.py
> > +++ b/gdb/copyright.py
> > @@ -148,15 +148,12 @@ def may_have_copyright_notice(filename):
> >      # so just open the file as a byte stream. We only need to search
> >      # for a pattern that should be the same regardless of encoding,
> >      # so that should be good enough.
> > -    fd = open(filename, "rb")
> > -
> > -    lineno = 1
> > -    for line in fd:
> > -        if b"Copyright" in line:
> > -            return True
> > -        lineno += 1
> > -        if lineno > 50:
> > -            return False
> > +    with open(filename, "rb") as fd:
> > +        for lineno, line in enumerate(fd, start=1):
> > +            if b"Copyright" in line:
> > +                return True
> > +            if lineno > 50:
> > +                break
> >      return False
> >  
> >  
> > -- 
> > 2.33.0
> > 



-- 
Joel

Re: [PATCH] gdb: copyright: make file header scan a bit more pythonic

[Simon Marchi]

On 2022-01-01 13:15, Mike Frysinger via Gdb-patches wrote:
> Should be functionally the same, but uses more pythonic idioms to get
> fewer lines of code, and to make sure to not leak open file handles.
> ---
>  gdb/copyright.py | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/copyright.py b/gdb/copyright.py
> index a78f7f2aa9b0..918d2e473d49 100755
> --- a/gdb/copyright.py
> +++ b/gdb/copyright.py
> @@ -148,15 +148,12 @@ def may_have_copyright_notice(filename):
>      # so just open the file as a byte stream. We only need to search
>      # for a pattern that should be the same regardless of encoding,
>      # so that should be good enough.
> -    fd = open(filename, "rb")
> -
> -    lineno = 1
> -    for line in fd:
> -        if b"Copyright" in line:
> -            return True
> -        lineno += 1
> -        if lineno > 50:
> -            return False
> +    with open(filename, "rb") as fd:
> +        for lineno, line in enumerate(fd, start=1):
> +            if b"Copyright" in line:
> +                return True
> +            if lineno > 50:

I was checking the warnings given by flake8:

copyright.py:143:5: F841 local variable 'MAX_LINES' is assigned to but never used

I think this `50` was meant to be MAX_LINES.  You might as well change
the code to use it.  LGTM in any case:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCH] gdb: copyright: make file header scan a bit more pythonic

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On 24 Oct 2022 21:16, Simon Marchi wrote:
> On 2022-01-01 13:15, Mike Frysinger via Gdb-patches wrote:
> > Should be functionally the same, but uses more pythonic idioms to get
> > fewer lines of code, and to make sure to not leak open file handles.
> > ---
> >  gdb/copyright.py | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/gdb/copyright.py b/gdb/copyright.py
> > index a78f7f2aa9b0..918d2e473d49 100755
> > --- a/gdb/copyright.py
> > +++ b/gdb/copyright.py
> > @@ -148,15 +148,12 @@ def may_have_copyright_notice(filename):
> >      # so just open the file as a byte stream. We only need to search
> >      # for a pattern that should be the same regardless of encoding,
> >      # so that should be good enough.
> > -    fd = open(filename, "rb")
> > -
> > -    lineno = 1
> > -    for line in fd:
> > -        if b"Copyright" in line:
> > -            return True
> > -        lineno += 1
> > -        if lineno > 50:
> > -            return False
> > +    with open(filename, "rb") as fd:
> > +        for lineno, line in enumerate(fd, start=1):
> > +            if b"Copyright" in line:
> > +                return True
> > +            if lineno > 50:
> 
> I was checking the warnings given by flake8:
> 
> copyright.py:143:5: F841 local variable 'MAX_LINES' is assigned to but never used
> 
> I think this `50` was meant to be MAX_LINES.  You might as well change
> the code to use it.  LGTM in any case:
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>

i had noticed that locally already and looks like i forgot to send out a v2
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]