[PATCH v2 1/2] Refactor sigcontextinfo.h

Florian Weimer fweimer@redhat.com
Thu Aug 15 14:07:00 GMT 2019


* Adhemerval Zanella:

> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/sigcontextinfo.h b/sysdeps/unix/sysv/linux/sparc/sparc32/sigcontextinfo.h
> index 9eec807a23..af1bcd21e2 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/sigcontextinfo.h
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/sigcontextinfo.h
> @@ -16,5 +16,38 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#define SIGCONTEXT struct sigcontext *
> -#define GET_PC(__ctx)	((void *) ((__ctx)->si_regs.pc))
> +#ifndef _SIGCONTEXTINFO_H
> +#define _SIGCONTEXTINFO_H
> +
> +/* The kernel sparc32 signal frame for SA_SIGINFO is layout as:

“is defined as”?

> +  struct rt_signal_frame32 {
> +   struct sparc_stackf32 ss;
> +   compat_siginfo_t info;
> +   struct pt_regs32 regs;
> +   compat_sigset_t mask;   
> +   u32 fpu_save;
> +   unsigned int insns[2];
> +   compat_stack_t stack;
> +   unsigned int extra_size;
> +   siginfo_extra_v8plus_t v8plus;
> +   u32 rwin_save;
> +  } __attribute__((aligned(8)));

Isn't it GNU style to put the { on a line by itself (unindenteed)?
This is also relevant to the code, see struct kernel_sigset_t a bit
below in the posted patch.

> +  And different that other architectures, SPARC32 pass the pt_regs32
> +  REGS pointer as third argument for sa_sigaction handler with 
> +  SA_SIGINFO.  */

“Unlike other architectures, sparc32 passes pt_regs32 REGS pointers as
the third argument to a sa_sigaction handler with SA_SIGINFO enabled.”
or something like that?

“Different that” appears in a few other places as well which also need
fixing.

> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/register-dump.h b/sysdeps/unix/sysv/linux/sparc/sparc64/register-dump.h
> index 0f1078ad9e..722f06e09d 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/register-dump.h
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/register-dump.h
> @@ -60,17 +60,36 @@ hexvalue (unsigned long int value, char *buf, size_t len)
>      *--cp = '0';
>  }
>  
> +/* Different that other architectures, SPARC32 pass a pt_regs (or pt_regs32
> +   in 32 bits compat mode) struct pointer as third argument for sa_sigaction
> +   handler with SA_SIGINFO.

I think this is the sparc64 header, so the comment needs to be adjusted.

> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigcontextinfo.h b/sysdeps/unix/sysv/linux/sparc/sparc64/sigcontextinfo.h
> index a2f2b1f7de..744b51c585 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigcontextinfo.h
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/sigcontextinfo.h
> @@ -16,8 +16,45 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#ifndef _SIGCONTEXTINFO_H
> +#define _SIGCONTEXTINFO_H
> +
> +#include <bits/types/siginfo_t.h>
> +
> +/* The kernel sparc64 signal frame for SA_SIGINFO is layout as:
> +
> +   struct rt_signal_frame {
> +     struct sparc_stackf ss;
> +     siginfo_t info;
> +     struct pt_regs regs; 
> +     __siginfo_fpu_t *fpu_save;
> +     stack_t stack;
> +     sigset_t mask;
> +     __siginfo_rwin_t *rwin_save; 
> +   };
> +
> +   And different that other architectures, SPARC64 pass the siginfo_t
> +   INFO pointer as third argument for sa_sigaction handler with 
> +   SA_SIGINFO.  */
> +
>  #ifndef STACK_BIAS
>  #define STACK_BIAS 2047
>  #endif
> -#define SIGCONTEXT struct sigcontext *
> -#define GET_PC(__ctx)	((void *) ((__ctx)->sigc_regs.tpc))
> +
> +struct pt_regs
> +{
> + unsigned long u_regs[16];
> + unsigned long tstate;
> + unsigned long tpc;
> + unsigned long tnpc;
> + unsigned int y;
> + unsigned int magic;
> +}; 

Wrong indentation, and “unsigned long int” should be used.

The patch has trailing whitespace in a few places, FWIW.

Overall, I'm in favor of this change.  I do wonder if GET_PC should be
an inline function instead, so that we get type checking for its
argument (even if the implementation needs to cast the pointer to
something else).

Thnaks,
Florian



More information about the Libc-alpha mailing list