[PATCH] bsd-user/signal.c: Only copy the _capsicum for FreeBSD_version > 1400026

Warner Losh posted 1 patch 2 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220201205251.57691-1-imp@bsdimp.com
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>
There is a newer version of this series
bsd-user/signal.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] bsd-user/signal.c: Only copy the _capsicum for FreeBSD_version > 1400026
Posted by Warner Losh 2 years, 3 months ago
The capsicum signal stuff is new with FreeBSD 14, rev 1400026, so only
copy them on a new enough system.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index ad22ba9d90d..b43266e6e06 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -242,8 +242,10 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
          * one, then we know what to save.
          */
         if (sig == TARGET_SIGTRAP) {
+#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
             tinfo->_reason._capsicum._syscall =
                 info->_reason._capsicum._syscall;
+#endif
             si_type = QEMU_SI_CAPSICUM;
         }
         break;
@@ -296,8 +298,10 @@ static void tswap_siginfo(target_siginfo_t *tinfo, const target_siginfo_t *info)
         __put_user(info->_reason._poll._band, &tinfo->_reason._poll._band);
         break;
     case QEMU_SI_CAPSICUM:
+#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
         __put_user(info->_reason._capsicum._syscall,
                    &tinfo->_reason._capsicum._syscall);
+#endif
         break;
     default:
         g_assert_not_reached();
-- 
2.33.1


Re: [PATCH] bsd-user/signal.c: Only copy the _capsicum for FreeBSD_version > 1400026
Posted by Warner Losh 2 years, 3 months ago
This fixes a build regression. The _capsicum member was added on -current
recently, and isn't in FreeBSD 12 or 13. This fixes the build regression
there.

My apologies...

Warner

On Tue, Feb 1, 2022 at 1:52 PM Warner Losh <imp@bsdimp.com> wrote:

> The capsicum signal stuff is new with FreeBSD 14, rev 1400026, so only
> copy them on a new enough system.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/signal.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index ad22ba9d90d..b43266e6e06 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -242,8 +242,10 @@ static inline void
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>           * one, then we know what to save.
>           */
>          if (sig == TARGET_SIGTRAP) {
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
>              tinfo->_reason._capsicum._syscall =
>                  info->_reason._capsicum._syscall;
> +#endif
>              si_type = QEMU_SI_CAPSICUM;
>          }
>          break;
> @@ -296,8 +298,10 @@ static void tswap_siginfo(target_siginfo_t *tinfo,
> const target_siginfo_t *info)
>          __put_user(info->_reason._poll._band,
> &tinfo->_reason._poll._band);
>          break;
>      case QEMU_SI_CAPSICUM:
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
>          __put_user(info->_reason._capsicum._syscall,
>                     &tinfo->_reason._capsicum._syscall);
> +#endif
>          break;
>      default:
>          g_assert_not_reached();
> --
> 2.33.1
>
>
Re: [PATCH] bsd-user/signal.c: Only copy the _capsicum for FreeBSD_version > 1400026
Posted by Richard Henderson 2 years, 3 months ago
On 2/2/22 07:52, Warner Losh wrote:
> The capsicum signal stuff is new with FreeBSD 14, rev 1400026, so only
> copy them on a new enough system.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/signal.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index ad22ba9d90d..b43266e6e06 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -242,8 +242,10 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>            * one, then we know what to save.
>            */
>           if (sig == TARGET_SIGTRAP) {
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
>               tinfo->_reason._capsicum._syscall =
>                   info->_reason._capsicum._syscall;
> +#endif
>               si_type = QEMU_SI_CAPSICUM;
>           }

Do you really want the entire IF inside the ifdef?
Setting QEMU_SI_CAPSICUM doesn't make sense for earlier versions...


r~

>           break;
> @@ -296,8 +298,10 @@ static void tswap_siginfo(target_siginfo_t *tinfo, const target_siginfo_t *info)
>           __put_user(info->_reason._poll._band, &tinfo->_reason._poll._band);
>           break;
>       case QEMU_SI_CAPSICUM:
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
>           __put_user(info->_reason._capsicum._syscall,
>                      &tinfo->_reason._capsicum._syscall);
> +#endif
>           break;
>       default:
>           g_assert_not_reached();


Re: [PATCH] bsd-user/signal.c: Only copy the _capsicum for FreeBSD_version > 1400026
Posted by Warner Losh 2 years, 3 months ago
On Tue, Feb 1, 2022 at 2:06 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/2/22 07:52, Warner Losh wrote:
> > The capsicum signal stuff is new with FreeBSD 14, rev 1400026, so only
> > copy them on a new enough system.
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/signal.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> > index ad22ba9d90d..b43266e6e06 100644
> > --- a/bsd-user/signal.c
> > +++ b/bsd-user/signal.c
> > @@ -242,8 +242,10 @@ static inline void
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> >            * one, then we know what to save.
> >            */
> >           if (sig == TARGET_SIGTRAP) {
> > +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
> >               tinfo->_reason._capsicum._syscall =
> >                   info->_reason._capsicum._syscall;
> > +#endif
> >               si_type = QEMU_SI_CAPSICUM;
> >           }
>
> Do you really want the entire IF inside the ifdef?
> Setting QEMU_SI_CAPSICUM doesn't make sense for earlier versions...
>

True. In fact, not defining it at all on earlier versions would be safer to
ensure
all the places where we might want to touch this we'll get a compiler
warning
as well as to communicate this is only for earlier versions w/o sprinkling
the
version through the code. I'll update with this suggestion.

Warner


>
> r~
>
> >           break;
> > @@ -296,8 +298,10 @@ static void tswap_siginfo(target_siginfo_t *tinfo,
> const target_siginfo_t *info)
> >           __put_user(info->_reason._poll._band,
> &tinfo->_reason._poll._band);
> >           break;
> >       case QEMU_SI_CAPSICUM:
> > +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1400026
> >           __put_user(info->_reason._capsicum._syscall,
> >                      &tinfo->_reason._capsicum._syscall);
> > +#endif
> >           break;
> >       default:
> >           g_assert_not_reached();
>
>