xiaoxiang781216 commented on PR #17357: URL: https://github.com/apache/nuttx/pull/17357#issuecomment-3562102642
> > > > The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged. > > > > > > > > > Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead. > > > > > > It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than #17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138 > > Bascially, we can arrange the pr into three part: > > > > 1. Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness. > > 2. One patch skip the signal dispatch in the common code > > 3. Final patch skip all signal related function > > > > Who volunteers to complete this work? @wangchdo or @extinguish. > > I don't think this PR is better because if signal support is disabled, then the related APIs should also be disabled. eg. the implementation of sleep/usleep needs to be redirected to the sched_sleep() version, and doesn't need to support signal handling. Could you review the patch carefully before making the comment? In this patch, you can still interrupt the target thread from sleep, but you can't trigger the signal handle. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
