improved SI_TIMER decoding

enh enh at google.com
Tue Mar 11 00:08:11 UTC 2014


On Mon, Mar 10, 2014 at 4:52 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Thu 06 Mar 2014 18:20:50 enh wrote:
>> Decode siginfo_t more clearly for si_code SI_TIMER.
>>
>> The 'pid' is actually a POSIX timer id, and the 'uid' is actually the
>> overrun.
>>
>> Also factor out the si_value dumping so it's the same for every si_code.
>
> please use `git send-email` rather than attaching (and doing base64 encoding
> of) your patches.  it makes it much harder to do reviews when it's like this.
>
>> @@ -561,32 +572,35 @@ printsiginfo(siginfo_t *sip, int verbose)
>>               }
>>  #ifdef SI_FROMUSER
>>               if (SI_FROMUSER(sip)) {
>> -                     tprintf(", si_pid=%lu, si_uid=%lu",
>> -                             (unsigned long) sip->si_pid,
>> -                             (unsigned long) sip->si_uid);
>>                       switch (sip->si_code) {
>>  #ifdef SI_USER
>>                       case SI_USER:
>> +                             tprintf(", si_pid=%lu, si_uid=%lu",
>> +                                     (unsigned long) sip->si_pid,
>> +                                     (unsigned long) sip->si_uid);
>>                               break;
>>  #endif
>>  #ifdef SI_TKILL
>>                       case SI_TKILL:
>> +                             tprintf(", si_pid=%lu, si_uid=%lu",
>> +                                     (unsigned long) sip->si_pid,
>> +                                     (unsigned long) sip->si_uid);
>>                               break;
>>  #endif
>
> well, now the output handling for source signals has been duplicated 3 times.
> that doesn't seem like an improvement.  why not split the logic for decoding
> of the source and decoding of the value so there's no duplication ?

because i don't think they're correct (except SI_TKILL). i think all
of these need to be revisited. see siginfo.h. i only fixed SI_TIMER
because that's the one i spent the week looking at, so that's the one
i can guarantee was wrong and is now right.

>>                       default:
>> +                     tprintf(", si_pid=%lu, si_uid=%lu",
>> +                             (unsigned long) sip->si_pid,
>> +                             (unsigned long) sip->si_uid);
>
> indent here is incorrect
> -mike




More information about the Strace-devel mailing list