Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file leak in QEMU tracer #138

Merged
merged 1 commit into from
Jul 21, 2023
Merged

Conversation

dnivra
Copy link
Contributor

@dnivra dnivra commented Apr 28, 2023

Currently, mktemp is used to get name of a temporary file to which the QEMU output file from taregt is copied to. However, this file is never cleaned up and mktemp seems to be deprecated. This PR replaces mktemp with a NamedTemporaryFile that is cleaned up at the end. All CI runners seem to have failing test cases on master so I'm guessing I didn't break anything with this change.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023

Test Results

  20 files  +10    20 suites  +10   25m 37s ⏱️ + 10m 6s
103 tests +21    76 ✔️ +21    26 💤 ±0  1 +1 
460 runs  +21  330 ✔️ +21  129 💤 ±0  1 +1 

For more details on these failures, see this check.

Results for commit abc6ada. ± Comparison against base commit fafe677.

♻️ This comment has been updated with latest results.

@Kyle-Kyle
Copy link
Contributor

can you rebase the PR and run the CI again? I'll try to fix the CI soon-ish.

@dnivra dnivra force-pushed the fix/fix-file-leak-in-qemu-tracer branch from 1132e3d to 509892c Compare July 16, 2023 06:24
@Kyle-Kyle
Copy link
Contributor

can you please rebase the PR? archr CI is finally fixed after so many years

@dnivra dnivra force-pushed the fix/fix-file-leak-in-qemu-tracer branch from 509892c to abc6ada Compare July 21, 2023 14:24
@Kyle-Kyle Kyle-Kyle merged commit 3e0100f into master Jul 21, 2023
@Kyle-Kyle Kyle-Kyle deleted the fix/fix-file-leak-in-qemu-tracer branch July 21, 2023 17:57
@Kyle-Kyle
Copy link
Contributor

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants