Date: Tue, 19 Nov 2024 22:01:33 -0700 From: Warner Losh <imp@bsdimp.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@freebsd.org> Subject: Re: git: c29cba408d19 - main - stand: Narrow the 'signal emulation' we provide Message-ID: <CANCZdfraUbASy_Oc8Zo5KBq7ViTgMBWJXy3aZgRLpXDTsmt3TQ@mail.gmail.com> In-Reply-To: <CBB3C014-A7FB-4758-A77E-AC35ABD88EAD@freebsd.org> References: <202411200335.4AK3ZnVU099504@gitrepo.freebsd.org> <CBB3C014-A7FB-4758-A77E-AC35ABD88EAD@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Tue, Nov 19, 2024, 8:56 PM Jessica Clarke <jrtc27@freebsd.org> wrote: > On 20 Nov 2024, at 03:35, Warner Losh <imp@FreeBSD.org> wrote: > > > > The branch main has been updated by imp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=c29cba408d197ed2c267d0605e4225bb54153e01 > > > > commit c29cba408d197ed2c267d0605e4225bb54153e01 > > Author: Warner Losh <imp@FreeBSD.org> > > AuthorDate: 2024-11-20 03:05:11 +0000 > > Commit: Warner Losh <imp@FreeBSD.org> > > CommitDate: 2024-11-20 03:24:41 +0000 > > > > stand: Narrow the 'signal emulation' we provide > > > > We only need to provide sig_atomic_t in emulation. However, including > > machine/signal.h brings in too much namespace pollution related to > > signals. Instead, define sig_atomic_t as long. Setting long is async > > atomic on all platforms (though powerpc64 defines it to an int), > though > > that doesn't matter since the boot loader doesn't use signals. > > > > Sponsored by: Netflix > > --- > > stand/libsa/stand.h | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/stand/libsa/stand.h b/stand/libsa/stand.h > > index 260defa3a33d..e1188fb73a26 100644 > > --- a/stand/libsa/stand.h > > +++ b/stand/libsa/stand.h > > @@ -82,8 +82,13 @@ > > #define EOFFSET (ELAST+8) /* relative seek not supported */ > > #define ESALAST (ELAST+8) /* */ > > > > -/* Partial signal emulation for sig_atomic_t */ > > -#include <machine/signal.h> > > +/* > > + * LUA needs sig_atomic_t. This is defined to be long or int on all our > > + * platforms. On all but powerpc, these are all the same thing as long. > 64-bit > > + * powerpc defines this as int, but long can also be accessed > atomically. It's > > + * also OK because we don't have signal handlers in the boot loader. > > + */ > > Presumably an oversight when adding the powerpc64* ports :( Probably we > should fix this by taking advantage of being able to break ABI for tier > 2 architectures between major versions. Shame it wasn’t done the same > time as ELFv1 -> ELFv2. We fixed riscv and arm64’s SIG_ATOMIC_MIN/MAX > recently, but the underlying types were still long in that case, > thankfully. > Likely. But i already have too much on my plate. Just a weird thing i noticed. Warner Jess > > > +typedef long sig_atomic_t; > > > > __BEGIN_DECLS > > > > [-- Attachment #2 --] <div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 19, 2024, 8:56 PM Jessica Clarke <<a href="mailto:jrtc27@freebsd.org">jrtc27@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 20 Nov 2024, at 03:35, Warner Losh <imp@FreeBSD.org> wrote:<br> > <br> > The branch main has been updated by imp:<br> > <br> > URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=c29cba408d197ed2c267d0605e4225bb54153e01" rel="noreferrer noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=c29cba408d197ed2c267d0605e4225bb54153e01</a><br>; > <br> > commit c29cba408d197ed2c267d0605e4225bb54153e01<br> > Author: Warner Losh <imp@FreeBSD.org><br> > AuthorDate: 2024-11-20 03:05:11 +0000<br> > Commit: Warner Losh <imp@FreeBSD.org><br> > CommitDate: 2024-11-20 03:24:41 +0000<br> > <br> > stand: Narrow the 'signal emulation' we provide<br> > <br> > We only need to provide sig_atomic_t in emulation. However, including<br> > machine/signal.h brings in too much namespace pollution related to<br> > signals. Instead, define sig_atomic_t as long. Setting long is async<br> > atomic on all platforms (though powerpc64 defines it to an int), though<br> > that doesn't matter since the boot loader doesn't use signals.<br> > <br> > Sponsored by: Netflix<br> > ---<br> > stand/libsa/stand.h | 9 +++++++--<br> > 1 file changed, 7 insertions(+), 2 deletions(-)<br> > <br> > diff --git a/stand/libsa/stand.h b/stand/libsa/stand.h<br> > index 260defa3a33d..e1188fb73a26 100644<br> > --- a/stand/libsa/stand.h<br> > +++ b/stand/libsa/stand.h<br> > @@ -82,8 +82,13 @@<br> > #define EOFFSET (ELAST+8) /* relative seek not supported */<br> > #define ESALAST (ELAST+8) /* */<br> > <br> > -/* Partial signal emulation for sig_atomic_t */<br> > -#include <machine/signal.h><br> > +/*<br> > + * LUA needs sig_atomic_t. This is defined to be long or int on all our<br> > + * platforms. On all but powerpc, these are all the same thing as long. 64-bit<br> > + * powerpc defines this as int, but long can also be accessed atomically. It's<br> > + * also OK because we don't have signal handlers in the boot loader.<br> > + */<br> <br> Presumably an oversight when adding the powerpc64* ports :( Probably we<br> should fix this by taking advantage of being able to break ABI for tier<br> 2 architectures between major versions. Shame it wasn’t done the same<br> time as ELFv1 -> ELFv2. We fixed riscv and arm64’s SIG_ATOMIC_MIN/MAX<br> recently, but the underlying types were still long in that case,<br> thankfully.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Likely. But i already have too much on my plate. Just a weird thing i noticed.</div><div dir="auto"><br></div><div dir="auto">Warner</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Jess<br> <br> > +typedef long sig_atomic_t;<br> > <br> > __BEGIN_DECLS<br> > <br> <br> </blockquote></div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfraUbASy_Oc8Zo5KBq7ViTgMBWJXy3aZgRLpXDTsmt3TQ>