Re: [PATCH] LoongArch: Ensure consistency with kernel by using union for struct members in mcontext_t and ucontext_t.

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

The 04/06/2023 17:59, caiyinyu wrote:
> 在 2023/4/5 上午1:38, Szabolcs Nagy 写道:
> > The 04/03/2023 20:01, caiyinyu wrote:
> > > During the construction of the LoongArch Alpine system,
> > > we found that there is an inconsistency in the member
> > > names of mcontext_t and ucontext_t between musl and glibc,
> > > which can cause compilation errors. After testing, we decided
> > > to use union to keep these member names consistency.
> > there is no musl api compat requirement at the moment (the musl
> > loongarch port is not committed yet so surely that can be
> > changed to be consistent).
> 
> 
> We want to ensure that the structure member names in mcontext_t are
> 
> consistent with the kernel (like other architectures)
> 
> before submitting LoongArch musl (Currently under development) to the
> upstream community.
> 
> This will allow for consistency in the structure member names
> 
> across musl, glibc, and the kernel for related structures.
> 
> 
> > there is an api difference between glibc and linux uapi headers.
> > i personally don't think this is an issue. (the c abi must match
> > but struct sigcontext and mcontext_t are not used interchangably
> > in code. the c++ mangling abi is different even after your patch)
> 
> 
> I am currently facing some confusion.
> 
> As far as I know, in C++, the member names of a struct or union are not
> subject to mangling.

sorry i did not mean to say you are breaking c++ abi compared to
the current glibc code, but that the linux sigcontext struct has
different c++ abi even if the field names are the same (because
sigcontext vs mcontext_t struct tag).

this tells me that you cannot use those two types interchangably
in c++ library apis anyway and i would expect that the two types
don't get confused in source code, so the field names don't need
to be consistent (of course it's nicer if they are the same).

one minor issue is that in principle this is a posix api and the
header is supposed to work with any c99 compiler, but unnamed
union and struct members are not c99. in practice i don't think
this is an issue: all loongarch c compilers will support it
and glibc already relies on extensions (like attribute aligned).

i'm not against the patch, but to me the commit message implied
the change was because of musl, while it is for being more
consistent with linux uapi and other targets.