userspace: Exclude nuttx/arch.h
Summary
The nuttx/arch.h included in userspace.h will cause some unnecessary headers to be introduced in the user program (such as net.h). We are trying to remove the inclusion of nuttx/arch.h. Also, the inclusion of nuttx/arch.h in user-space headers was causing issues because architecture-related definitions and APIs should not be exposed to users. By removing this inclusion, the patch ensures that user-space applications do not have access to or rely on architecture-specific details, which is crucial for maintaining a clean separation between user-space and kernel-space.
Impact
The impact of this change is primarily on the user-space applications that were previously using nuttx/arch.h. These applications will now need to be updated to remove the dependency on this header. The change should not affect the build process, hardware, documentation, or security.
Testing
Tested on QEMU/arm64 and QEMU/x86_64.
[Experimental Bot, please feedback here]
The PR seems to meet most of the NuttX requirements. However, there are some areas that could be improved:
Summary:
- Good: Provides a clear explanation of the problem (
nuttx/arch.hinclusion brings unnecessary headers and exposes architecture-specific details) and the proposed solution (removing the inclusion). - Missing:
- Functional part: Specify the exact files and functions modified by the change. For instance, mention that the change modifies
include/nuttx/userspace.hand removes the#include <nuttx/arch.h>line. - How it works: While you explain the "what," elaborating on the "how" would strengthen the summary. Briefly describe the mechanism used to remove the dependency on
nuttx/arch.hwithout breaking user-space applications.
- Functional part: Specify the exact files and functions modified by the change. For instance, mention that the change modifies
Impact:
- Good: Clearly states the impact on user-space applications and mentions no impact on other aspects.
- Needs improvement:
- User impact: Be more specific about how users need to adapt. Do they need to include a different header? Modify their code? Provide concrete examples if possible.
- Compatibility: You mention no impact, but removing a header file can lead to compatibility issues. Clarify if this change guarantees backward compatibility. If not, specify the versions or scenarios affected.
Testing:
- Good: Mentions the tested architectures (QEMU/arm64, QEMU/x86_64).
- Missing:
- Build Host: Specify the operating system and compiler used for testing.
- Target (board:config): Mention the specific boards and configurations used for testing on each architecture.
- Logs: The template requests "testing logs before change" and "testing logs after change." While not strictly required for every PR, providing relevant log snippets demonstrating the issue before the change and the successful outcome after the change would significantly strengthen the testing section.
In summary: The PR addresses an important issue and provides a good starting point. However, adding the missing details will make it even stronger and easier for reviewers to understand and approve.
Sorry @Fix-Point: Could you take a look at esp32s3-devkit:eth_lan9250, is it failing because of this PR? Thanks!
Configuration/Tool: esp32s3-devkit/eth_lan9250
chip/esp32s3_efuse_lowerhalf.c: In function 'esp32s3_efuse_lowerhalf_write':
Error: chip/esp32s3_efuse_lowerhalf.c:142:11: error: implicit declaration of function 'enter_critical_section' [-Werror=implicit-function-declaration]
142 | flags = enter_critical_section();
| ^~~~~~~~~~~~~~~~~~~~~~
Error: chip/esp32s3_efuse_lowerhalf.c:152:3: error: implicit declaration of function 'leave_critical_section' [-Werror=implicit-function-declaration]
152 | leave_critical_section(flags);
| ^~~~~~~~~~~~~~~~~~~~~~
https://github.com/NuttX/nuttx/actions/runs/11630100298/job/32388417948#step:7:864
Also freedom-k64f:demo and mikroe-stm32f4:fulldemo. Thanks!
https://github.com/NuttX/nuttx/actions/runs/11630100298/job/32388417242#step:7:859
Configuration/Tool: freedom-k64f/demo,CONFIG_ARM_TOOLCHAIN_GNU_EABI
Error: sensors/fxos8700cq.c:96:39: error: 'struct file' declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
96 | static int fxos8700cq_open(FAR struct file *filep);
| ^~~~
https://github.com/NuttX/nuttx/actions/runs/11630100298/job/32388420301#step:7:523
Configuration/Tool: mikroe-stm32f4/fulldemo,CONFIG_ARM_TOOLCHAIN_GNU_EABI
audio/vs1053.c: In function 'vs1053_stop':
Error: audio/vs1053.c:1579:3: error: implicit declaration of function 'up_mdelay' [-Werror=implicit-function-declaration]
1579 | up_mdelay(40);
Also
freedom-k64f:demoandmikroe-stm32f4:fulldemo. Thanks!https://github.com/NuttX/nuttx/actions/runs/11630100298/job/32388417242#step:7:859
Configuration/Tool: freedom-k64f/demo,CONFIG_ARM_TOOLCHAIN_GNU_EABI Error: sensors/fxos8700cq.c:96:39: error: 'struct file' declared inside parameter list will not be visible outside of this definition or declaration [-Werror] 96 | static int fxos8700cq_open(FAR struct file *filep); | ^~~~https://github.com/NuttX/nuttx/actions/runs/11630100298/job/32388420301#step:7:523
Configuration/Tool: mikroe-stm32f4/fulldemo,CONFIG_ARM_TOOLCHAIN_GNU_EABI audio/vs1053.c: In function 'vs1053_stop': Error: audio/vs1053.c:1579:3: error: implicit declaration of function 'up_mdelay' [-Werror=implicit-function-declaration] 1579 | up_mdelay(40);
https://github.com/apache/nuttx/pull/14603/commits
Sorry @Fix-Point I think we have a similar problem with esp32s3-devkit/eth_lan9250. FYI I tracked this using our new NuttX Build History Tool
Configuration/Tool: esp32s3-devkit/eth_lan9250
chip/esp32s3_efuse_lowerhalf.c: In function 'esp32s3_efuse_lowerhalf_write':
Error: chip/esp32s3_efuse_lowerhalf.c:142:11: error: implicit declaration of function 'enter_critical_section' [-Werror=implicit-function-declaration]
142 | flags = enter_critical_section();
| ^~~~~~~~~~~~~~~~~~~~~~
Error: chip/esp32s3_efuse_lowerhalf.c:152:3: error: implicit declaration of function 'leave_critical_section' [-Werror=implicit-function-declaration]
152 | leave_critical_section(flags);
| ^~~~~~~~~~~~~~~~~~~~~~
https://github.com/NuttX/nuttx/actions/runs/11630100298/job/32388417948#step:7:864
Fix here: https://github.com/apache/nuttx/pull/14610, @lupyuen
this pr will lead compile error make distclean -j20; ./tools/configure.sh -l omnibusf4:nsh ;make -j20
CC: ctype/lib_tolower.c mtd/sector512.c: In function 's512_ioctl': mtd/sector512.c:554:19: error: invalid use of undefined type 'struct partition_info_s' 554 | info->numsectors = priv->neblocks * priv->stdperblock; | ^~ mtd/sector512.c:555:19: error: invalid use of undefined type 'struct partition_info_s' 555 | info->sectorsize = SECTOR_512; | ^~ mtd/sector512.c:556:19: error: invalid use of undefined type 'struct partition_info_s' 556 | info->startsector = 0; | ^~ mtd/sector512.c:557:19: error: invalid use of undefined type 'struct partition_info_s' 557 | info->parent[0] = '\0';
@hujun260 Fix here: https://github.com/apache/nuttx/pull/14609