improved SI_TIMER decoding

[enh]

i think we might be talking at cross purposes. the pid/uid stuff that
isn't relevant for SI_TIMER isn't in sigval_t; it's in siginfo_t. i
did factor out the sigval_t stuff.

On Mon, Mar 10, 2014 at 5:37 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Mon 10 Mar 2014 17:08:11 enh wrote:
>> 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:
>> >>  #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.
>
> i don't follow your logic.  why should we merge duplicate tprintf calls now
> when it's trivial to split & not duplicate ?
>
>         /* Decode source details when it makes sense. */
>         switch (sip->si_code) {
>         ... cases that decode pid/uid ...
>         ... other source details ...
>         }
>
>         /* Decode sigval_t when it makes sense. */
>         switch (sip->si_code) {
>         ... only list cases where this field is valid ...
>         }
>
> although it looks like the patch has already been merged (with the style issue
> fixed), so now it's a matter of cleaning up the new code.
> -mike



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Java i18n/JNI/NIO, or bionic questions? Mail me/drop by/add me as a reviewer.