clean up how internal APIs are used

[zephyrbot]

Reported by Hirally Santiago:

Right now internal APIs are not consistently specified.

1. Some are placed in header files.
2. Some are not in any header, and code that wants to use them needs to drop function prototypes in their C code.

#2 is horrible. If these APIs need to change, all the prototypes need to be corrected. This is why header files exist in the first place.
Take inventory of our internal APIs and put them in headers as appropriate.

(Imported from Jira ZEP-56)

[zephyrbot]

by Inaky Perez-Gonzalez:

The following shall be the standard to follow:

• include/AREA.h or include/AREA/*.h: public APIs related to driver or subsystem AREA. Any kernel driver or subsystem, or application can use them.
  -- include/arch/ARCH: reserver for architecture specific APIs
• PATH/AREA/*.h: private APIs related to driver or subsystem AREA: only files part of the subsystem or driver AREA can access and use them.

Rules:

• All header files should be protected from double inclusion by declaring their relative path as a #define, excluding the include component (this is done to avoid namespace clashes between files with the same name in different subsystems):

#ifndef __[SUBDIR_]FILENAME_H__
#define __[SUBDIR_]FILENAME_H__

... declarations ...

#endif /* !__[SUBDIR_]FILENAME_H__ */

• A header file should include all the header files it depends on (it would compile cleanly if included solely in an empty .c file).

[zephyrbot]

by Inaky Perez-Gonzalez:

Peter Mitsis , Andrew Boie can you guys provide some feedback on this before I take it to the mailing list?

[zephyrbot]

by Peter Mitsis:

Three comments.

If I am following the proposal correctly, the FILENAME_H will contain the relative path. I like the idea. When include files get moved or renamed, we will have to remember to modify FILENAME_H. The tree seems relatively stable for now, and I do not forsee this being a significant issue. However, in light of the upcoming unified kernel work, I think that it is worthwhile to get some additional feedback from Al Stephens as he is taking charge of that while Ben is away.

It's minor, but for the comment associated with the closing #endif, I would prefer that it simply be /* !__FILENAME_H */. It is less wordy, and we already have precedence.

Are there any expected exceptions to the "always put the extern declarations in a header file" rule? An example for consideration ... __do_global_ctors_aux(). The routines k_init.c::_main() and nano_init.c::_main() declare it inside their respective routines as ...
extern void __do_global_ctors_aux(void);
... where it is used there and only there. It conveys a sense of locality that is lost with putting the external declaration in a header file. It could very well be that the benefits of this ?fringe? case are too small to be worrying over. However, I wanted to mention it in case there is adequate value.

[zephyrbot]

by Inaky Perez-Gonzalez:

(1) added notes about the subsystem thing. Allan Stephens , can you please chime in?
(2) good point, modified

(3)

• I am of the opinion that "extern" is not really needed (just the func declaration)
• misc/cpp_ctors.c is exporting an interface, so in all propriety:

void __do_global_ctors_aux(void);
void __do_init_array_aux(void);

should be in include/cpp-ctors.h.
There is also the question on this being a very internal kernel interface. In Linux this is handled by having three interfaces:
-- internal (more or less include/.h
-- user level (in include/uapi/.h)
-- syscall interface (more or less include/linux/*.h)

I see the case for having a place for internal kernel interfaces, another one for public interfaces. I would lay it out similar to Linux's:

include/[SUBSYSTEM/]*.h internal
include/api/[SUBSYSTEM/]*.h external?

but I hate this naming scheme...

So then, that file could be in include/cpp/ctors.h, for example. But then, that being an internal API, drivers and apps in general would not see it.

[zephyrbot]

by Peter Mitsis:

No objections from me.

[zephyrbot]

by Allan Stephens:

I think the proposal makes sense, and don't have any objections to folks moving forward with it. However, since the upcoming unified kernel work will make significant changes to the internals of the "kernel" subsystem, and to some of the files under "include", it may make sense to wait for those changes to occur first and then do any remaining cleanup. Otherwise, you run the risk of making changes that then get thrown away.

A few things to consider:

1. How urgent is it do get this cleanup done? Can we delay this work until the Zephyr 1.6 time frame to allow the main parts of the unified kernel to be put in place?

2. How many internal APIs are there that need to be cleaned up? If there are only a handful, there's probably no reason not to do the changes right away -- if there are a lot, then delaying things is probably the better way to go.

[zephyrbot]

by Sharron LIU:

Public APIs or header files re-org, without adding/removal any public APIs.
Testability review mark "NotTest".

All existing kernel tests will detect any regressions/side-effects introduced by this re-work.
This clean up will make the test/app developer more easy to distinguish public APIs from internal APIs. And the cleanup may introduce changes on binary size, this will be detected by benchmark test accordingly.

[zephyrbot]

by Inaky Perez-Gonzalez:

Allan Stephens I agree this makes sense to delay until after the kernel merge

Sharron LIU the whole re-work itself will also affect public APIs, which is what we have to figure out how to do to minimize the impact (or make it easy to change to).

[zephyrbot]

by Sharron LIU:

Inaky Perez-Gonzalez , when mentioning "the whole re-work itself will also affect public APIs" do you mean any public APIs adding/removal?
If only move public APIs or header files from here to there, no new test cases introduced.
But, if any internal APIs goes public, or any public APIs added/removed, test cases need adapt to that. Thanks.

[zephyrbot]

by Inaky Perez-Gonzalez:

Sharron LIU we might move things around to make it consistent; it is hard to tell from the get go what is going to be the best way around it, as there will be case-by -case decisions. As the micro/nano kernel merge might already be changing a big chunk of the Public API (Allan Stephens ?), it might be a good moment to move it all.

[zephyrbot]

by Andrew Boie:

If Inaky Perez-Gonzalez is busy I can work on this one for 1.6.

[zephyrbot]

by Mark Linkmeyer:

Fixing incorrect priority

[nashif]

old and obsolete, wont fix