Re: [PATCH] for POSIX timer code

[george anzinger <george at mvista dot com>]

Andreas Jaeger wrote:
> 
> george anzinger <george@mvista.com> writes:
> 
> > Having set info.si_code to SI_TIMER it seems inappropriate to then set
> > union fields that relate to some other field.  Here we suggest a change
> > to set the timer1 field to the timer ID.  This is compatible with the
> > high-res-timers code which we hope to get in the kernel.
> 
> But doesn't this change current behaviour?  Is the changed behaviour
> according to the standards?  Changing glibc to conform to your
> implementation is not a good reason for me. 

I am tying to get more detail on what the standard has to say on this
subject, but it seems to be hidden somewhere other than in the timer_*
discussions.  For starters, however, it is clear to me and the writer of
the current glibc routine that info.si_code should be SI_TIMER.  This
then, IMHO, means that one should use the union member that relates to
timers which does not contain, either in the kernel or the library
header files, either si_uid or si_pid, entries which really do not make
any sense for a timer pop.

Still, I am searching both my copy of the standard and to see if I
should get another, more comprehensive copy.
> 
> > It seem highly unlikely that anyone would be using this info in the
> > current implementation.
> 
> George, thanks for the submission.  Even when Ulrich has to review it
> properly, let me add some comments on style.  In glibc we use the GNU
> Coding style rules which means that code should be indented correctly,
> and documented properly:
> - Please send next time a ChangeLog entry (as plain text)
> 
> > -g
> >
> >
> > ---
> > /usr/src/incoming/glibc/glibc-2.2.5-hr/linuxthreads/sysdeps/pthread/timer_routines.c~
> > Sat Apr 21 13:42:42 2001
> > +++
> > /usr/src/incoming/glibc/glibc-2.2.5-hr/linuxthreads/sysdeps/pthread/timer_routines.c
> > Thu Mar  7 16:16:55 2002
> > @@ -333,8 +333,7 @@
> >             value.  */
> >       info.si_signo = timer->event.sigev_signo;
> >       info.si_code = SI_TIMER;
> > -     info.si_pid = timer->creator_pid;
> > -     info.si_uid = getuid ();
> > +        info.si_timer1 = timer_ptr2id(timer);
> 
> - Use tabs instead of spaces (check the lines you removed and the one
>   you added) at the beginning
I wonder if cut and paste messed this up?  Most of the code I am looking
at (clock_ and timer_) seems to use two spaces for indention.  I MUCH
prefer the tab as that is what is used in the kernel.
> - Add a space before a brace

What is the convention about "(".  I prefer to keep it close to the
function name, but I see a lot of "one space before" code.
> - btw. if you like to have your code in glibc, we need a proper
>   copyright assignment.  Ask me off-list for details.
> 
> Thanks,
> Andreas
> --
>  Andreas Jaeger
>   SuSE Labs aj@suse.de
>    private aj@arthur.inka.de
>     http://www.suse.de/~aj

-- 
George           george@mvista.com
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Real time sched: http://sourceforge.net/projects/rtsched/