Re: [PATCH v2 3/4] linux-user: fix TARGET_NSIG and _NSIG uses

[Laurent Vivier]

Le 11/02/2020 à 17:47, Peter Maydell a écrit :
> On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <address@hidden> wrote:
>>
>> Valid signal numbers are between 1 (SIGHUP) and SIGRTMAX.
>>
>> System includes define _NSIG to SIGRTMAX + 1, but
>> QEMU (like kernel) defines TARGET_NSIG to TARGET_SIGRTMAX.
>>
>> Fix all the checks involving the signal range.
>>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>>
>> Notes:
>>     v2: replace i, j by target_sig, host_sig
>>
>>  linux-user/signal.c | 52 ++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 37 insertions(+), 15 deletions(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index 246315571c09..c1e664f97a7c 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -30,6 +30,15 @@ static struct target_sigaction sigact_table[TARGET_NSIG];
> 
> Optional follow-on patch: make sigact_table[] also size
> TARGET_NSIG + 1, for consistency with target_to_host_signal_table[],
> and remove all the "- 1"s when we index into it.
> 

OK,

>> @@ -492,10 +514,10 @@ static void signal_table_init(void)
>>          if (host_to_target_signal_table[host_sig] == 0) {
>>              host_to_target_signal_table[host_sig] = host_sig;
>>          }
>> -    }
>> -    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
>>          target_sig = host_to_target_signal_table[host_sig];
>> -        target_to_host_signal_table[target_sig] = host_sig;
>> +        if (target_sig <= TARGET_NSIG) {
>> +            target_to_host_signal_table[target_sig] = host_sig;
>> +        }
> 
> Why does this hunk apparently delete the for() line ?

It effectively deletes the for() line because I merge the two "for
(host_sig = 1; host_sig < _NSIG; host_sig++)" loops into one.

> Why do we need the if() -- surely there should never be any
> entries in host_to_target_signal_table[] that aren't
> valid target signal numbers ?
> 

we have above the "host_to_target_signal_table[host_sig] = host_sig;"
and host_sig can be greater than TARGET_NSIG.

Setting like this allows to ignore them later in the target as we can
compare them to TARGET_NSIG. This mapping 1:1 in the default case is the
original behaviour.

Thanks,
Laurent