Re: [PATCH 12/44] metag: Use get_signal() signal_setup_done()

[James Hogan]

Hi Richard,

On 02/03/14 23:57, Richard Weinberger wrote:
> @@ -235,10 +230,8 @@ static void handle_signal(unsigned long sig, siginfo_t *info,
>  static int do_signal(struct pt_regs *regs, int syscall)
>  {
>  	unsigned int retval = 0, continue_addr = 0, restart_addr = 0;
> -	struct k_sigaction ka;
> -	siginfo_t info;
> -	int signr;
>  	int restart = 0;
> +	struct ksignal ksig;
>  
>  	/*
>  	 * By the end of rt_sigreturn the context describes the point that the
> @@ -272,30 +265,30 @@ static int do_signal(struct pt_regs *regs, int syscall)
>  	}
>  
>  	/*
> -	 * Get the signal to deliver. When running under ptrace, at this point
> -	 * the debugger may change all our registers ...
> -	 */
> -	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
> -	/*
>  	 * Depending on the signal settings we may need to revert the decision
>  	 * to restart the system call. But skip this if a debugger has chosen to
>  	 * restart at a different PC.
>  	 */
>  	if (regs->REG_PC != restart_addr)
>  		restart = 0;
> -	if (signr > 0) {
> +
> +	/*
> +	 * Get the signal to deliver. When running under ptrace, at this point
> +	 * the debugger may change all our registers ...
> +	 */
> +	if (get_signal(&ksig)) {

The patch all looks good to me except for this bit. You've moved the bit
at which a ptrace debugger can alter the registers to after the check
for whether the PC has been altered, which would still need to cancel
the restart.

Does something like the hunks below look reasonable to you?

Would you like me to take this patch through the metag tree or do you
want to keep them together?

Cheers
James

@@ -275,7 +268,7 @@ static int do_signal(struct pt_regs *regs, int syscall)
 	 * Get the signal to deliver. When running under ptrace, at this point
 	 * the debugger may change all our registers ...
 	 */
-	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
+	get_signal(&ksig);
 	/*
 	 * Depending on the signal settings we may need to revert the decision
 	 * to restart the system call. But skip this if a debugger has chosen to
@@ -283,19 +276,19 @@ static int do_signal(struct pt_regs *regs, int syscall)
 	 */
 	if (regs->REG_PC != restart_addr)
 		restart = 0;
-	if (signr > 0) {
+	if (ksig.sig > 0) {
 		if (unlikely(restart)) {
 			if (retval == -ERESTARTNOHAND
 			    || retval == -ERESTART_RESTARTBLOCK
 			    || (retval == -ERESTARTSYS
-				&& !(ka.sa.sa_flags & SA_RESTART))) {
+				&& !(ksig.ka.sa.sa_flags & SA_RESTART))) {
 				regs->REG_RETVAL = -EINTR;
 				regs->REG_PC = continue_addr;
 			}
 		}
 
 		/* Whee! Actually deliver the signal.  */
-		handle_signal(signr, &info, &ka, regs);
+		handle_signal(&ksig, regs);
 		return 0;
 	}

Attachment: signature.asc
Description: OpenPGP digital signature