Re: [musl] [PATCH v2] riscv64: correct struct __ucontext name

[Rich Felker <dalias@libc.org>]

On Sun, Dec 06, 2020 at 05:10:25PM +0000, Ariadne Conill wrote:
> 
> On Sunday, December 6, 2020 10:06:49 AM MST Rich Felker wrote:
> > On Sun, Dec 06, 2020 at 04:55:39PM +0000, Ariadne Conill wrote:
> > > Hello,
> > > 
> > > On Sunday, December 6, 2020 5:49:25 AM MST Drew DeVault wrote:
> > > > On Sun Dec 6, 2020 at 3:51 AM EST, Szabolcs Nagy wrote:
> > > > > * Drew DeVault <sir@cmpwn.com> [2020-12-05 18:10:06 +0000]:
> > > > > > This makes it consistent with other architectures and fixes some
> > > > > > issues
> > > > > > with downstream software.
> > > > > 
> > > > > which software?
> > > > > 
> > > > > glibc uses struct ucontext_t too and user code should use ucontext_t
> > > > > without struct.
> > > 
> > > Some glibc architecture ports use the struct __ucontext and even struct
> > > ucontext names, or at least did in the past.
> > > 
> > > > libucontext, which does use ucontext_t.
> > > > 
> > > > In fact, the issue was more related to the type conflict with
> > > > ucontext.h, which declared struct __ucontext in the scope of its
> > > > function declarations due to the naming mismatch.
> > > 
> > > glibc uses the POSIX 2004 standardized ucontext_t type in its public
> > > definitions.  I believe musl should do the same.
> > 
> > This produces a compile-time error is ucontext.h is included without
> > the right feature test macros, since signal.h will not have defined
> > ucontext_t in that case. That's why the public declarations must use
> > the struct tag.
> 
> Bummer.  In that case, I suggest musl use the same struct tag consistently. It 
> should probably be struct ucontext_t for consistency with glibc.

I don't recall the original reason it was inconsitent with glibc here.
Note that signal.h has (which is probably wrong and should be
removed):

#ifdef _GNU_SOURCE
#define __ucontext ucontext
#endif

so that the tag gets renamed if _GNU_SOURCE is defined. Maybe at one
point glibc called it "struct ucontext", which was a namespace
violation since it's not in a reserved namespace, and we used
__ucontext to get the freedom to rename it and expose that only in a
_GNU_SOURCE context.

Whatever the reason, though, the struct tags is C++ABI and should not
be changed. The rv64 inconsitency was unintentional and hopefully is
not yet widely used enough for anyone to care that it's being fixed
now (in theory could mess up linking C++ libs with ucontext_t in their
public interface surface).

Note that none of this is visible to applications that aren't doing
anything horribly wrong. Neither "struct ucontext_t" nor "struct
__ucontext" (nor "struct ucontext", which it looks like we were
wrongly trying to support) is API.

Rich