search

ClangBuiltLinux / boot-utils

Collection of files for booting Linux kernels
26 stars 7 forks source link

boot-qemu.py: fix ctrl+c in gdb #87

Closed nickdesaulniers closed 1 year ago

nickdesaulniers commented 1 year ago

Detach QEMU from the process group so that SIGINT isn't also delivered to it.

Ignore SIGINT in python when running GDB subprocess.

Link: https://stackoverflow.com/a/13737455 Link: https://stackoverflow.com/q/30925198 Fixes: https://github.com/ClangBuiltLinux/boot-utils/issues/86

nickdesaulniers commented 1 year ago

The linter isn't happy.

boot-qemu.py:768:17: W1509: Using preexec_fn keyword which may be unsafe in the presence of threads (subprocess-popen-preexec-fn)

Looks like I might be able to use process_group=1 in the subprocess.Popen constructor, but seems it's only available in python 3.11+. (I'm on 3.10.8).

Should we disable W1509?

nathanchance commented 1 year ago

Should we disable W1509?

Yes, either in actions-workflow as you did or just adding # pylint: disable-next=subprocess-popen-preexec-fn above subprocess.Popen() here. Does not matter to me, pick your poison; just make sure everything is green before merging.

nathanchance commented 1 year ago

You can drop the pylint comment now since you merged https://github.com/ClangBuiltLinux/actions-workflows/commit/20ba4b599aef4d6b0bc2e122c398cf4638d32f72. It looks like there is just one outstanding yapf warning, then this should be good to go.