improved SI_TIMER decoding

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140310/c4743ca4/attachment.bin>