Greg,
Thanks. The commit link is there:
https://github.com/apache/nuttx/commit/e6227e19433c4999d500437d0a8a5c05f476ceb1
Here is the pull request
https://github.com/apache/nuttx/pull/8623
This pull request was written and reviewed by xiaomi (again) and only
ONE independent review (thanks for that).
No sane testing environment allows the same people to both write and
review code ! This is not how to quality.
If there are not enough independent reviewers, the pull requests should
be either BLOCKED or sent to the mailing list for help reviewing.
This would give a useful role to the mailing list.
There are lots of changes incoming to the repository, it is not humanly
possible to review them as well as Greg did in the past.
I have a dayjob, I can be called out to help if need be, but I cant
spend all my days checking everything with a critical point of view.
Issues have to be communicated so we can have a look at them.
BTW, what is wrong is this exact removal :
https://github.com/apache/nuttx/commit/e6227e19433c4999d500437d0a8a5c05f476ceb1#diff-944d6d12b7cba69e3ea374135ebc541fd819308865868374776e7dc79d627195L150
The condition by CONFIG_SCHED_HAVE_PARENT was removed. I cant find where
the inconditional code was added. in a separate commit maybe. However,
if that config is not set, SIGCHLD is defined by a constant in signal.h
and not by CONFIG_SIGCHLD, as seen here:
https://github.com/apache/nuttx/commit/e6227e19433c4999d500437d0a8a5c05f476ceb1#diff-944d6d12b7cba69e3ea374135ebc541fd819308865868374776e7dc79d627195R222
Here the Kconfig shows the dependency:
https://github.com/apache/nuttx/commit/e6227e19433c4999d500437d0a8a5c05f476ceb1#diff-285d196ad580d55fdaef3fbe54702ee3ddeed0d93cba52a5441d1866e0031726R1553
Sebastien
On 3/7/23 14:55, Gregory Nutt wrote:
It is likely that the issue with the SIGNAL number was introduced by
commit e6227e19433c4999d500437d0a8a5c05f476ceb1
Author: chenrun1 <chenr...@xiaomi.com>
Date: Wed Feb 1 17:17:43 2023 +0800
include/signal.h:Expanding SIGNAL to be consistent with Linux
There is nothing inherently wrong with the commit. However, none of
the default configurations were changed. Since the config->defconfig
compression removes all settings that are left at the default value,
changing the default value of a configuration setting effectively
changes the value used by the configuration.
When the default value of a configuration setting changes in a
Kconfig, NuttX uses should get some kind of heads up in the dev list
so that they can all fix their local configurations as well.
