Re: [Qemu-devel] [PATCH v2 03/28] linux-user: Reindent signal handling

[Laurent Vivier]

Le 24/05/2016 à 08:47, Riku Voipio a écrit :
> On tiistaina 24. toukokuuta 2016 3.21.34 EEST, Laurent Vivier wrote:
>>
>> Le 12/05/2016 à 19:47, Peter Maydell a écrit :
>>> From: Timothy E Baldwin 
>>>
>>> Some of the signal handling was a mess with a mixture of tabs and 8
>>> space
>>> indents.
>>
>> And adds some braces, but not everywhere.
>> [and fails on checkpatch.pl]
> 
> Reviewing this for unexpected changes is interesting. Found the most
> reasonable way was to compare the original and patched signal.c after
> passing them with "indent" using arguments roughly resembling qemu
> style[1]. This shows the added braces, which as far as I see don't
> change the logic.

Apply the patch, and "git show -w" shows you only non-space changes,
"git show -w --word-diff" gives you the exact changes.

>> If we don't add braces everywhere, perhaps it's better to add them
>> nowhere.
> 
> I think I will merge this as is, since this would mean fallout in
> editing the patches that come on top of this patch.

I have no problem with that.

Reviewed-by: Laurent Vivier 

BTW, I think signal.c should be split in the different target
directories under linux-user (I should have patch somewhere for that).

>> A coccinelle script can help to add them later.
>>
>> Laurent
>>
>>> Signed-off-by: Timothy Edward Baldwin
>>> 
>>> Message-id:
>>> 1441497448-32489-3-git-send-email-t.e.baldwi...@members.leeds.ac.uk
>>> Reviewed-by: Peter Maydell 
>>> [PMM: just rebased]
>>> Signed-off-by: Peter Maydell  ...
> 
> [1] indent -nbad -bap -nbc -bbo -hnl -br -brs -c33 -cd33 -ncdb -ce -ci4 
> -cli0  -d0 -di1 -nfc1 -i4 -ip0 -l80 -lp -npcs -nprs -npsl -sai -saf -saw
> -ncs -nsc -sob -nfca -cp33 -ss -ts4 -il1 -nut # kernel style changed to
> 4 spaces
> 
> 
Laurent

Re: [Qemu-devel] [PATCH v2 03/28] linux-user: Reindent signal handling

[Riku Voipio]

On tiistaina 24. toukokuuta 2016 3.21.34 EEST, Laurent Vivier wrote:


Le 12/05/2016 à 19:47, Peter Maydell a écrit :

From: Timothy E Baldwin 

Some of the signal handling was a mess with a mixture of tabs and 8 space
indents.


And adds some braces, but not everywhere.
[and fails on checkpatch.pl]


Reviewing this for unexpected changes is interesting. Found the most 
reasonable way was to compare the original and patched signal.c after 
passing them with "indent" using arguments roughly resembling qemu 
style[1]. This shows the added braces, which as far as I see don't change 
the logic.



If we don't add braces everywhere, perhaps it's better to add them nowhere.


I think I will merge this as is, since this would mean fallout in editing 
the patches that come on top of this patch.



A coccinelle script can help to add them later.

Laurent


Signed-off-by: Timothy Edward Baldwin 
Message-id: 
1441497448-32489-3-git-send-email-t.e.baldwi...@members.leeds.ac.uk

Reviewed-by: Peter Maydell 
[PMM: just rebased]
Signed-off-by: Peter Maydell  ...


[1] indent -nbad -bap -nbc -bbo -hnl -br -brs -c33 -cd33 -ncdb -ce -ci4  
-cli0  -d0 -di1 -nfc1 -i4 -ip0 -l80 -lp -npcs -nprs -npsl -sai -saf -saw 
-ncs -nsc -sob -nfca -cp33 -ss -ts4 -il1 -nut # kernel style changed to 4 
spaces

Re: [Qemu-devel] [PATCH v2 03/28] linux-user: Reindent signal handling

[Laurent Vivier]

Le 12/05/2016 à 19:47, Peter Maydell a écrit :
> From: Timothy E Baldwin 
> 
> Some of the signal handling was a mess with a mixture of tabs and 8 space
> indents.

And adds some braces, but not everywhere.
[and fails on checkpatch.pl]

If we don't add braces everywhere, perhaps it's better to add them nowhere.

A coccinelle script can help to add them later.

Laurent

> Signed-off-by: Timothy Edward Baldwin 
> Message-id: 
> 1441497448-32489-3-git-send-email-t.e.baldwi...@members.leeds.ac.uk
> Reviewed-by: Peter Maydell 
> [PMM: just rebased]
> Signed-off-by: Peter Maydell 
> ---
>  linux-user/signal.c | 1543 
> ++-
>  1 file changed, 791 insertions(+), 752 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 96e86c0..04c21d0 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -157,7 +157,7 @@ static void target_to_host_sigset_internal(sigset_t *d,
>  if (target_sigismember(s, i)) {
>  sigaddset(d, target_to_host_signal(i));
>  }
> - }
> +}
>  }
>  
>  void target_to_host_sigset(sigset_t *d, const target_sigset_t *s)
> @@ -250,18 +250,18 @@ static inline void 
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>  tinfo->si_code = info->si_code;
>  
>  if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
> -|| sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
> +|| sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
>  /* Should never come here, but who knows. The information for
> the target is irrelevant.  */
>  tinfo->_sifields._sigfault._addr = 0;
>  } else if (sig == TARGET_SIGIO) {
>  tinfo->_sifields._sigpoll._band = info->si_band;
> - tinfo->_sifields._sigpoll._fd = info->si_fd;
> +tinfo->_sifields._sigpoll._fd = info->si_fd;
>  } else if (sig == TARGET_SIGCHLD) {
>  tinfo->_sifields._sigchld._pid = info->si_pid;
>  tinfo->_sifields._sigchld._uid = info->si_uid;
>  tinfo->_sifields._sigchld._status
> -= host_to_target_waitstatus(info->si_status);
> += host_to_target_waitstatus(info->si_status);
>  tinfo->_sifields._sigchld._utime = info->si_utime;
>  tinfo->_sifields._sigchld._stime = info->si_stime;
>  } else if (sig >= TARGET_SIGRTMIN) {
> @@ -269,7 +269,7 @@ static inline void 
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>  tinfo->_sifields._rt._uid = info->si_uid;
>  /* XXX: potential problem if 64 bit */
>  tinfo->_sifields._rt._sigval.sival_ptr
> -= (abi_ulong)(unsigned long)info->si_value.sival_ptr;
> += (abi_ulong)(unsigned long)info->si_value.sival_ptr;
>  }
>  }
>  
> @@ -723,75 +723,75 @@ int do_sigaction(int sig, const struct target_sigaction 
> *act,
>  /* from the Linux kernel */
>  
>  struct target_fpreg {
> - uint16_t significand[4];
> - uint16_t exponent;
> +uint16_t significand[4];
> +uint16_t exponent;
>  };
>  
>  struct target_fpxreg {
> - uint16_t significand[4];
> - uint16_t exponent;
> - uint16_t padding[3];
> +uint16_t significand[4];
> +uint16_t exponent;
> +uint16_t padding[3];
>  };
>  
>  struct target_xmmreg {
> - abi_ulong element[4];
> +abi_ulong element[4];
>  };
>  
>  struct target_fpstate {
> - /* Regular FPU environment */
> -abi_ulong   cw;
> -abi_ulong   sw;
> -abi_ulong   tag;
> -abi_ulong   ipoff;
> -abi_ulong   cssel;
> -abi_ulong   dataoff;
> -abi_ulong   datasel;
> - struct target_fpreg _st[8];
> - uint16_tstatus;
> - uint16_tmagic;  /* 0x = regular FPU data only */
> -
> - /* FXSR FPU environment */
> -abi_ulong   _fxsr_env[6];   /* FXSR FPU env is ignored */
> -abi_ulong   mxcsr;
> -abi_ulong   reserved;
> - struct target_fpxreg_fxsr_st[8];/* FXSR FPU reg data is ignored 
> */
> - struct target_xmmreg_xmm[8];
> -abi_ulong   padding[56];
> +/* Regular FPU environment */
> +abi_ulong cw;
> +abi_ulong sw;
> +abi_ulong tag;
> +abi_ulong ipoff;
> +abi_ulong cssel;
> +abi_ulong dataoff;
> +abi_ulong datasel;
> +struct target_fpreg _st[8];
> +uint16_t  status;
> +uint16_t  magic;  /* 0x = regular FPU data only */
> +
> +/* FXSR FPU environment */
> +abi_ulong _fxsr_env[6];   /* FXSR FPU env is ignored */
> +abi_ulong mxcsr;
> +abi_ulong reserved;
> +struct target_fpxreg _fxsr_st[8]; /* FXSR FPU reg data is ignored */
> +struct target_xmmreg _xmm[8];
> +abi_ulong padding[56];
>  };
>  
>  #define X86_FXSR_MAGIC   0x
>  
>  

[Qemu-devel] [PATCH v2 03/28] linux-user: Reindent signal handling

[Peter Maydell]

From: Timothy E Baldwin 

Some of the signal handling was a mess with a mixture of tabs and 8 space
indents.

Signed-off-by: Timothy Edward Baldwin 
Message-id: 1441497448-32489-3-git-send-email-t.e.baldwi...@members.leeds.ac.uk
Reviewed-by: Peter Maydell 
[PMM: just rebased]
Signed-off-by: Peter Maydell 
---
 linux-user/signal.c | 1543 ++-
 1 file changed, 791 insertions(+), 752 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 96e86c0..04c21d0 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -157,7 +157,7 @@ static void target_to_host_sigset_internal(sigset_t *d,
 if (target_sigismember(s, i)) {
 sigaddset(d, target_to_host_signal(i));
 }
- }
+}
 }
 
 void target_to_host_sigset(sigset_t *d, const target_sigset_t *s)
@@ -250,18 +250,18 @@ static inline void 
host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
 tinfo->si_code = info->si_code;
 
 if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
-|| sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
+|| sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
 /* Should never come here, but who knows. The information for
the target is irrelevant.  */
 tinfo->_sifields._sigfault._addr = 0;
 } else if (sig == TARGET_SIGIO) {
 tinfo->_sifields._sigpoll._band = info->si_band;
-   tinfo->_sifields._sigpoll._fd = info->si_fd;
+tinfo->_sifields._sigpoll._fd = info->si_fd;
 } else if (sig == TARGET_SIGCHLD) {
 tinfo->_sifields._sigchld._pid = info->si_pid;
 tinfo->_sifields._sigchld._uid = info->si_uid;
 tinfo->_sifields._sigchld._status
-= host_to_target_waitstatus(info->si_status);
+= host_to_target_waitstatus(info->si_status);
 tinfo->_sifields._sigchld._utime = info->si_utime;
 tinfo->_sifields._sigchld._stime = info->si_stime;
 } else if (sig >= TARGET_SIGRTMIN) {
@@ -269,7 +269,7 @@ static inline void 
host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
 tinfo->_sifields._rt._uid = info->si_uid;
 /* XXX: potential problem if 64 bit */
 tinfo->_sifields._rt._sigval.sival_ptr
-= (abi_ulong)(unsigned long)info->si_value.sival_ptr;
+= (abi_ulong)(unsigned long)info->si_value.sival_ptr;
 }
 }
 
@@ -723,75 +723,75 @@ int do_sigaction(int sig, const struct target_sigaction 
*act,
 /* from the Linux kernel */
 
 struct target_fpreg {
-   uint16_t significand[4];
-   uint16_t exponent;
+uint16_t significand[4];
+uint16_t exponent;
 };
 
 struct target_fpxreg {
-   uint16_t significand[4];
-   uint16_t exponent;
-   uint16_t padding[3];
+uint16_t significand[4];
+uint16_t exponent;
+uint16_t padding[3];
 };
 
 struct target_xmmreg {
-   abi_ulong element[4];
+abi_ulong element[4];
 };
 
 struct target_fpstate {
-   /* Regular FPU environment */
-abi_ulong   cw;
-abi_ulong   sw;
-abi_ulong   tag;
-abi_ulong   ipoff;
-abi_ulong   cssel;
-abi_ulong   dataoff;
-abi_ulong   datasel;
-   struct target_fpreg _st[8];
-   uint16_tstatus;
-   uint16_tmagic;  /* 0x = regular FPU data only */
-
-   /* FXSR FPU environment */
-abi_ulong   _fxsr_env[6];   /* FXSR FPU env is ignored */
-abi_ulong   mxcsr;
-abi_ulong   reserved;
-   struct target_fpxreg_fxsr_st[8];/* FXSR FPU reg data is ignored 
*/
-   struct target_xmmreg_xmm[8];
-abi_ulong   padding[56];
+/* Regular FPU environment */
+abi_ulong cw;
+abi_ulong sw;
+abi_ulong tag;
+abi_ulong ipoff;
+abi_ulong cssel;
+abi_ulong dataoff;
+abi_ulong datasel;
+struct target_fpreg _st[8];
+uint16_t  status;
+uint16_t  magic;  /* 0x = regular FPU data only */
+
+/* FXSR FPU environment */
+abi_ulong _fxsr_env[6];   /* FXSR FPU env is ignored */
+abi_ulong mxcsr;
+abi_ulong reserved;
+struct target_fpxreg _fxsr_st[8]; /* FXSR FPU reg data is ignored */
+struct target_xmmreg _xmm[8];
+abi_ulong padding[56];
 };
 
 #define X86_FXSR_MAGIC 0x
 
 struct target_sigcontext {
-   uint16_t gs, __gsh;
-   uint16_t fs, __fsh;
-   uint16_t es, __esh;
-   uint16_t ds, __dsh;
-abi_ulong edi;
-abi_ulong esi;
-abi_ulong ebp;
-abi_ulong esp;
-abi_ulong ebx;
-abi_ulong edx;
-abi_ulong ecx;
-abi_ulong eax;
-abi_ulong trapno;
-abi_ulong err;
-abi_ulong eip;
-   uint16_t cs, __csh;
-abi_ulong eflags;
-abi_ulong esp_at_signal;
-