[PATCH] scripts: generate_rust_analyzer.py: avoid FD leak

Tamir Duberstein posted 1 patch 2 weeks, 2 days ago
There is a newer version of this series
scripts/generate_rust_analyzer.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
Posted by Tamir Duberstein 2 weeks, 2 days ago
Use a context manager to avoid leaking file descriptors.

Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Trevor Gross <tmgross@umich.edu>
Signed-off-by: Tamir Duberstein <tamird@kernel.org>
---
 scripts/generate_rust_analyzer.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 3b645da90092..5ed375c8aa3f 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -190,7 +190,8 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs, core_edit
 
     def is_root_crate(build_file, target):
         try:
-            return f"{target}.o" in open(build_file).read()
+            with open(build_file) as f:
+                return f"{target}.o" in f.read()
         except FileNotFoundError:
             return False
 

---
base-commit: 2af6ad09fc7dfe9b3610100983cccf16998bf34d
change-id: 20260122-rust-analyzer-fd-leak-b247830d666e

Best regards,
--  
Tamir Duberstein <tamird@kernel.org>
Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
Posted by Miguel Ojeda 1 week, 5 days ago
On Thu, Jan 22, 2026 at 5:44 PM Tamir Duberstein <tamird@kernel.org> wrote:
>
> Use a context manager to avoid leaking file descriptors.

This may have been intentionally written like that for simplicity,
since I think CPython closes them immediately in practice even if it
does not guarantee it (and I think the kernel may be assuming CPython
given the version requirement?).

Nevertheless, it is better to be explicit and proper, but it is not
urgent, so I would say let's put this in rust-analyzer after the merge
window even if you end up considering it a fix.

Like in the other one, I don't see the Tested-by from Daniel, so I
would suggest taking the chance to double-check that meanwhile too.

Thanks!

Cheers,
Miguel
Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
Posted by Tamir Duberstein 1 week, 4 days ago
On Sun, Jan 25, 2026 at 9:09 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Thu, Jan 22, 2026 at 5:44 PM Tamir Duberstein <tamird@kernel.org> wrote:
> >
> > Use a context manager to avoid leaking file descriptors.
>
> This may have been intentionally written like that for simplicity,
> since I think CPython closes them immediately in practice even if it
> does not guarantee it (and I think the kernel may be assuming CPython
> given the version requirement?).

I'm not sure how CPython could close the FD immediately - it would
require the GC to run, at least? Anyway, agree with you below:

> Nevertheless, it is better to be explicit and proper, but it is not
> urgent, so I would say let's put this in rust-analyzer after the merge
> window even if you end up considering it a fix.

Works for me.

> Like in the other one, I don't see the Tested-by from Daniel, so I
> would suggest taking the chance to double-check that meanwhile too.

I think you're right. I'll strip that tag.

> Thanks!

Thanks for reviewing!
Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
Posted by Levi Zim 1 week, 4 days ago
On 1/26/26 10:09 AM, Miguel Ojeda wrote:
> On Thu, Jan 22, 2026 at 5:44 PM Tamir Duberstein <tamird@kernel.org> wrote:
>>
>> Use a context manager to avoid leaking file descriptors.
> 
> This may have been intentionally written like that for simplicity,
> since I think CPython closes them immediately in practice even if it
> does not guarantee it (and I think the kernel may be assuming CPython
> given the version requirement?).

Path.read_text from pathlib would be a better choice for keeping the simplicity
while ensuring the file is closed.

https://docs.python.org/3/library/pathlib.html#pathlib.Path.read_text

Best regards,
Levi
 
> Nevertheless, it is better to be explicit and proper, but it is not
> urgent, so I would say let's put this in rust-analyzer after the merge
> window even if you end up considering it a fix.
> 
> Like in the other one, I don't see the Tested-by from Daniel, so I
> would suggest taking the chance to double-check that meanwhile too.
> 
> Thanks!
> 
> Cheers,
> Miguel
> 

Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
Posted by Tamir Duberstein 1 week, 4 days ago
On Tue, Jan 27, 2026 at 8:10 AM Levi Zim <i@kxxt.dev> wrote:
>
>
> On 1/26/26 10:09 AM, Miguel Ojeda wrote:
> > On Thu, Jan 22, 2026 at 5:44 PM Tamir Duberstein <tamird@kernel.org> wrote:
> >>
> >> Use a context manager to avoid leaking file descriptors.
> >
> > This may have been intentionally written like that for simplicity,
> > since I think CPython closes them immediately in practice even if it
> > does not guarantee it (and I think the kernel may be assuming CPython
> > given the version requirement?).
>
> Path.read_text from pathlib would be a better choice for keeping the simplicity
> while ensuring the file is closed.
>
> https://docs.python.org/3/library/pathlib.html#pathlib.Path.read_text

Thanks, I thought this would change semantics because `open` would
default to binary, but it defaults to text. I'll use read_text in v2.

>
> Best regards,
> Levi
>
> > Nevertheless, it is better to be explicit and proper, but it is not
> > urgent, so I would say let's put this in rust-analyzer after the merge
> > window even if you end up considering it a fix.
> >
> > Like in the other one, I don't see the Tested-by from Daniel, so I
> > would suggest taking the chance to double-check that meanwhile too.
> >
> > Thanks!
> >
> > Cheers,
> > Miguel
> >
>