scripts/generate_rust_analyzer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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>
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
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!
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 >
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 > > >
© 2016 - 2026 Red Hat, Inc.