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

[James Hogan]

On 03/03/14 11:15, James Hogan wrote:
> 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?

I've applied the altered version of this patch to the metag tree for v3.15.

Thanks
James

> 
> 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