[multi-arch-dev] doc: Add initial multi-arch-coding-guideline #8789
Conversation
yfliuuu
force-pushed
the
coding-guideline
branch
from
770dd79 to
d336ef8
Compare
multi-arch-coding-guideline.rst
Outdated
| <arch member1>; | ||
| <arch member2>; | ||
| <arch member3>; | ||
| struct foo *opaque_foo; |
There was a problem hiding this comment.
Can we leverage container_of to avoid this pointer?
There was a problem hiding this comment.
The example here was not very appropriate. I'll add another.
Yes we can use container_of when we tried to get outer struct with inner struct pointer.
My original point is that, if a struct is referenced as opaque pointer, we need a forward declaration.
multi-arch-coding-guideline.rst
Outdated
|
|
||
| /* Forward declaration when the reference of struct is opaque */ | ||
| struct foo; | ||
| struct bar; |
There was a problem hiding this comment.
I haven't got the point here, why can not include the common.h directly?
There was a problem hiding this comment.
Common header includes arch-public header first, which means the arch-public content expands before common header. If arch-public again includes common, that part simply becomes empty because of the outer header guard from common header.
If arch-public header needs to reference contents in common (opaque reference), a forward declaration is needed.
A simple example is, a common vcpu header defines struct vcpu, and arch-public vcpu header has API that needs to take struct vcpu * as input.
However if arch header needs to explicitly reference members inside struct vcpu (maybe through inline function), then this creates circular dependency. There are usually two solutions: 1, do not inline this function and put implementation to C file, 2, that function is NOT an arch API and it should not be placed in arch-public header.
| * Define __ARCH_HAS_OPTIONAL_WITH_DEFAULT in arch header to | ||
| * indicate implementation instead. | ||
| */ | ||
| void arch_api_optional_with_default(void); |
There was a problem hiding this comment.
The arch_ is a prefix for arch depended implementation. If there's a default generic implementation that should be without arch_ prefix. You can refer to the patch I wrote for Haoyu.
There was a problem hiding this comment.
My understanding is: arch_ functions are functions that are supposed to be implemented by arch domain. However some arch may choose to do it, some arch don't. If they choose to do it, the implementation is in arch, and it is correct to start function with arch_.
Having a default in common domain doesn't mean that it is not an arch_ function.
KVM has the same logic. KVM has some arch_ default API implemented in common C file (for example kvm_arch_create_vm_debugfs).
| =================== | ||
|
|
||
| This is the most common form of interface where a function is referenced in common scope | ||
| and implemented in arch-specific scope. |
There was a problem hiding this comment.
We need to empower the principle is that arch/x86/* should only implement common API with arch_ prefix, never implement a common API directly. If a common API doesn't have any generic code logic, then wrap the arch_xxx as a simple function in common.h like your helper case.
There was a problem hiding this comment.
Sure, will emphasize this.
yfliuuu
force-pushed
the
coding-guideline
branch
7 times, most recently
from
274d48c to
043fad9
Compare
|
start to build |
yfliuuu
force-pushed
the
coding-guideline
branch
from
043fad9 to
96958fa
Compare
|
start to build |
Tracked-On: projectacrn#8782 Signed-off-by: Yifan Liu <[email protected]>
4 participants
Tracked-On: #8782