-
Notifications
You must be signed in to change notification settings - Fork 647
apps: decouple signal support #3217
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
base: master
Are you sure you want to change the base?
Conversation
fc21c7e to
9845726
Compare
1. sigprocmask_test/sighand_test/signest_test/suspend_test: these testcases are using to test the signal api 2. sigev_thread_test: this testcase need to access signal related function 3. timer_test: this testcase need to access signal related function Signed-off-by: guoshichao <[email protected]>
make the nsh tools can work with SIGNAL disabled Signed-off-by: guoshichao <[email protected]>
using pthread_cancel to implement the exit logic, thus to make the netutils tools can work when signal disabled Signed-off-by: guoshichao <[email protected]>
using the local cu_globals_s instance to manange the cu exit procedure Signed-off-by: guoshichao <[email protected]>
merge the cu.h to cu_main.c, to make the cu tools code more cleaner Signed-off-by: guoshichao <[email protected]>
The core functionality of the cu tool does not rely on signal features. Therefore, we can isolate the signal-related implementations to ensure that most functions of the cu tool remain operational even when signals are disabled. Signed-off-by: guoshichao <[email protected]>
9845726 to
67dd5d2
Compare
The commit 'add support for CONFIG_DISABLE_SIGNALS' led to: undefined reference to `signal' undefined reference to `sigaction' Signed-off-by: v-tangmeng <[email protected]>
…GNALS The commit 'add support for CONFIG_DISABLE_SIGNALS' led to: undefined reference to `sigaction' Signed-off-by: v-tangmeng <[email protected]>
cederom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @extinguish great work!
This PR is a prototype for testing and goes in pair with apache/nuttx#17352. Needs careful review and testing. When parent PR is merged this one goes in pair :-)
I am just marking RC here not to merge by accident :-)
@cederom this patch need be merged first to unlock ci error on apache/nuttx#17352. If the community accept to add DISABLE_SIGNALS option, it's better to merge this simple pr first, so we can ensure the kernel side change don't break any build. |
|
@xiaoxiang781216 Do you know if @extinguish is doing it coordinated with @wangchdo ? Otherwise we will have many issues. |
@extinguish 's userspace change is the subset of @wangchdo 's since @wangchdo disable the full signal feature. |
|
If the community agree to disable signal with option, it's better to merge this pr first, so both @extinguish and @wangchdo 's change can move forward and verify with ci. @cederom @acassis |
ab0054a to
69ee29e
Compare
|
@extinguish there are some errors, i.e.: |
c97bb67
this is fixed in updated pull request |
|
@extinguish but the same error still report. |
Got it, I'm fixing it now, this is a new issue introduced by supporting the protect build process for mksym_tab.c. |
c97bb67 to
15ca04b
Compare
the "examples/elf project refactor and disable signal procedure" is separate to a new pull request, and the new pull request is based on #3223, after #3223 is review passed and merged, the examples/elf refactor pull request will upstreaming |
Summary
Depencies: apache/nuttx#17352.
Added support for signal decoupling, primarily involving modifications to the following modules in the apps: ostest, nsh/tools, netutils, and the cu tool.
CONFIG_DISABLE_SIGNALS=y, certain tests that rely on signal functionality need to be disabled.CONFIG_DISABLE_SIGNALS=y.Impact
No adjustments were made to the functional logic; support was only added for when
CONFIG_DISABLE_SIGNALS=y.Testing