-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Change all function prefix from up_ to arch_ in include/nuttx/arch.h #915
Comments
The best step is: |
The naming convenions required that global function platform fucntions .. that is, functions used outside of arch/ and board/ MUST begion with up. Functions used only within arch/ and board/ must NOT begin with up_. See https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ arch_ is never permitted as a function prefix. |
In #916, the error is in the name up_fullcontextrestore(tcb->xcp.regs); which is incorrect. That should be arm_fullcontextrestore(). The naming in other ARM architectures is incorrect. The other architectures are not following the naming standard. ARMv8-M is correctly following the naming standard. So yes, there will be issues UNTIL other ARM architectures are mad to follow the naming standard. Let's do that!! Let's fix the other ARM architectures and not screw up the naming standard. |
As you state in https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ: |
We just want to change up_xxx function common across armv7-a, armv7-m... to arm_xxx, but up_ is also used for the common function across arm, riscv..., so it is impossible to replace all up_ inside arch/arm to arm_ auotmaitcally. |
I have started the renaming with PR #924 After that is merged I will do more, a little at a time. |
Common functions across RISC-V (only) should begin with riscv_, not up and not up_ or arch_ There is not interaction between the private functions in ARM and the private functions in other archtectures such as RISC-V. That is proper modular design. The naming must all be private and internale to each architecture and should NOT be shared. |
Private functions used only within arch/ and board/ for the same MCU should never have the prefix up_ (or arch_) and should never be exposed via arch.h. That is very bad modular thinking and we must not even consider that. up_ is intended for common MCU interfaces that are used by the OS, usually in sched/ and must always have the prefix up_ and must always be prototypes in include/nuttx/arch.h. I disagree with you completely and think that this is a terrible idea. It is not good modular thinking and we must not consider it. |
You misunderstand my meaning, let me give an example: |
@xiaoxiang781216 I have put a lot of effort in the past few days and submitted several PRs to bring the arch/arm directories into compliance with the naming convention of https://cwiki.apache.org/confluence/display/NUTTX/Naming+FAQ I think if we follow the naming convention, we should not run into any future issues like #916 |
Sure, let's clean up arm arch first, and see whether change up_ to arch_ for all function inside arch.h can help rename other arch more efficiently. |
I just want to make sure that we protect the modular architecture. I don't want to expose ARM internal functions though arch.h. That would not be a good decision. Just changing the naming from up_ to arch_ is not so important (but effects a lot of code, lots of documentation, and the Wiki). It is probably better to stay wtih the prefix up_ for historic reasons. |
The internal architecture interfaces are not documented and, hence, have no impact outside of the architecture-specific code and the architecture-specific documentation. The up_ functions has global scope are part of the well-controlled, modular interface. The naming and location where the function is prototyped is a critical part of the modular architecutre. arch.h defines the common, generic interface between ALL archtectures and the rest of the OS. |
@patacongo does this make sense?
The text was updated successfully, but these errors were encountered: