public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
@ 2026-01-22 16:44 Tamir Duberstein
  2026-01-26  2:09 ` Miguel Ojeda
  0 siblings, 1 reply; 6+ messages in thread
From: Tamir Duberstein @ 2026-01-22 16:44 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Alex Gaynor, Fiona Behrens,
	Boris-Chengbiao Zhou
  Cc: Kees Cook, rust-for-linux, linux-kernel, stable, Daniel Almeida,
	Tamir Duberstein

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>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
  2026-01-22 16:44 [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak Tamir Duberstein
@ 2026-01-26  2:09 ` Miguel Ojeda
  2026-01-27 13:09   ` Levi Zim
  2026-01-27 13:38   ` Tamir Duberstein
  0 siblings, 2 replies; 6+ messages in thread
From: Miguel Ojeda @ 2026-01-26  2:09 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Alex Gaynor, Boris-Chengbiao Zhou, Kees Cook,
	rust-for-linux, linux-kernel, stable, Daniel Almeida,
	Fiona Behrens

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
  2026-01-26  2:09 ` Miguel Ojeda
@ 2026-01-27 13:09   ` Levi Zim
  2026-01-27 13:38     ` Tamir Duberstein
  2026-01-27 13:38   ` Tamir Duberstein
  1 sibling, 1 reply; 6+ messages in thread
From: Levi Zim @ 2026-01-27 13:09 UTC (permalink / raw)
  To: Miguel Ojeda, Tamir Duberstein
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Alex Gaynor, Boris-Chengbiao Zhou, Kees Cook,
	rust-for-linux, linux-kernel, stable, Daniel Almeida,
	Fiona Behrens


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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
  2026-01-26  2:09 ` Miguel Ojeda
  2026-01-27 13:09   ` Levi Zim
@ 2026-01-27 13:38   ` Tamir Duberstein
  2026-02-11 13:26     ` Miguel Ojeda
  1 sibling, 1 reply; 6+ messages in thread
From: Tamir Duberstein @ 2026-01-27 13:38 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Alex Gaynor, Boris-Chengbiao Zhou, Kees Cook,
	rust-for-linux, linux-kernel, stable, Daniel Almeida,
	Fiona Behrens

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!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
  2026-01-27 13:09   ` Levi Zim
@ 2026-01-27 13:38     ` Tamir Duberstein
  0 siblings, 0 replies; 6+ messages in thread
From: Tamir Duberstein @ 2026-01-27 13:38 UTC (permalink / raw)
  To: Levi Zim
  Cc: Miguel Ojeda, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Alex Gaynor, Boris-Chengbiao Zhou,
	Kees Cook, rust-for-linux, linux-kernel, stable, Daniel Almeida,
	Fiona Behrens

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
> >
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak
  2026-01-27 13:38   ` Tamir Duberstein
@ 2026-02-11 13:26     ` Miguel Ojeda
  0 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2026-02-11 13:26 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Alex Gaynor, Boris-Chengbiao Zhou, Kees Cook,
	rust-for-linux, linux-kernel, stable, Daniel Almeida,
	Fiona Behrens

On Tue, Jan 27, 2026 at 2:39 PM Tamir Duberstein <tamird@kernel.org> wrote:
>
> 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:

At least it looked to be immediately closed from a quick test I did.

I assume it knows because the temporary goes away and thus the
refcount goes to zero, without needing a GC run:

  https://docs.python.org/3/reference/datamodel.html

> Thanks for reviewing!

You're welcome!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-11 13:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-22 16:44 [PATCH] scripts: generate_rust_analyzer.py: avoid FD leak Tamir Duberstein
2026-01-26  2:09 ` Miguel Ojeda
2026-01-27 13:09   ` Levi Zim
2026-01-27 13:38     ` Tamir Duberstein
2026-01-27 13:38   ` Tamir Duberstein
2026-02-11 13:26     ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox