I concur with Gedare's comments. I would think your commit message would be similar to the subject of the ticket. This looks like a decent example:
https://git.rtems.org/rtems/commit/?id=6c23252cdd8ea63d0fe13d9f99b7befbf070fe80 Please update and submit. Pay attention to compiler warnings. On Fri, Jul 23, 2021 at 12:02 PM Gedare Bloom <ged...@rtems.org> wrote: > > Hi Zak, > > Please provide a useful first commit line. I think I've mentioned this > before, but the current guidance is found at > https://devel.rtems.org/wiki/Developer/Git#GitCommits > > Put the ticket closing line within the commit message, usually on its > own line at the end of your commit message > > > On Thu, Jul 22, 2021 at 6:29 PM Zacchaeus Leung > <zakthertems...@gmail.com> wrote: > > > > --- > > cpukit/include/rtems/posix/timer.h | 1 + > > cpukit/posix/src/psxtimercreate.c | 1 + > > cpukit/posix/src/timergettime.c | 61 +++++++++++++++++++----------- > > 3 files changed, 41 insertions(+), 22 deletions(-) > > > > diff --git a/cpukit/include/rtems/posix/timer.h > > b/cpukit/include/rtems/posix/timer.h > > index bcbf07a65a..7ae089173a 100644 > > --- a/cpukit/include/rtems/posix/timer.h > > +++ b/cpukit/include/rtems/posix/timer.h > > @@ -48,6 +48,7 @@ typedef struct { > > uint32_t ticks; /* Number of ticks of the initialization */ > > uint32_t overrun; /* Number of expirations of the timer */ > > struct timespec time; /* Time at which the timer was started */ > > + clockid_t clock_type; /* The type of timer */ > > } POSIX_Timer_Control; > > > > /** > > diff --git a/cpukit/posix/src/psxtimercreate.c > > b/cpukit/posix/src/psxtimercreate.c > > index a63cf1d100..804c7a41e7 100644 > > --- a/cpukit/posix/src/psxtimercreate.c > > +++ b/cpukit/posix/src/psxtimercreate.c > > @@ -91,6 +91,7 @@ int timer_create( > > ptimer->timer_data.it_value.tv_nsec = 0; > > ptimer->timer_data.it_interval.tv_sec = 0; > > ptimer->timer_data.it_interval.tv_nsec = 0; > > + ptimer->clock_type = clock_id; > > > > _Watchdog_Preinitialize( &ptimer->Timer, _Per_CPU_Get_snapshot() ); > > _Watchdog_Initialize( &ptimer->Timer, _POSIX_Timer_TSR ); > > diff --git a/cpukit/posix/src/timergettime.c > > b/cpukit/posix/src/timergettime.c > > index ee2a566f0e..ff2176ac60 100644 > > --- a/cpukit/posix/src/timergettime.c > > +++ b/cpukit/posix/src/timergettime.c > > @@ -28,6 +28,7 @@ > > #include <rtems/score/todimpl.h> > > #include <rtems/score/watchdogimpl.h> > > #include <rtems/seterr.h> > > +#include <rtems/timespec.h> > > > > /* > > * - When a timer is initialized, the value of the time in > > @@ -35,39 +36,55 @@ > > * - When this function is called, it returns the difference > > * between the current time and the initialization time. > > */ > > - > avoid random whitespace changes. > > > int timer_gettime( > > timer_t timerid, > > struct itimerspec *value > > -) > > +) > avoid random whitespace changes. I can't even tell what this one changes. > > > { > > POSIX_Timer_Control *ptimer; > > - ISR_lock_Context lock_context; > > - uint64_t now; > > - uint32_t remaining; > > + ISR_lock_Context lock_context; > > + uint32_t remaining; > > + Per_CPU_Control *cpu; > > + struct timespec *now; > > + struct timespec *expire; > > + struct timespec *result; > > > > - if ( !value ) > > - rtems_set_errno_and_return_minus_one( EINVAL ); > > + if (!value) > > + rtems_set_errno_and_return_minus_one(EINVAL); > avoid whitespace changes. This change is inconsistent with our coding > conventions. > > > > > - ptimer = _POSIX_Timer_Get( timerid, &lock_context ); > > - if ( ptimer != NULL ) { > > - Per_CPU_Control *cpu; > > + ptimer = _POSIX_Timer_Get(timerid, &lock_context); > avoid whitespace changes. This change is inconsistent with our coding > conventions. > > > + if (ptimer == NULL) { > add spaces after ( and before ), e.g., > if ( ptimer == NULL ) { > ^ ^ > > > + rtems_set_errno_and_return_minus_one(EINVAL); > Add spaces around EINVAL > > > + } > > > > - cpu = _POSIX_Timer_Acquire_critical( ptimer, &lock_context ); > > - now = cpu->Watchdog.ticks; > > + cpu = _POSIX_Timer_Acquire_critical(ptimer, &lock_context); > avoid whitespace changes. This change is inconsistent with our coding > conventions. > > > + rtems_timespec_from_ticks(ptimer->Timer.expire, expire); > This isn't correct. expire is an uninitialized pointer. How do you > compile and run this code? I'm surprised it passes tests with this > kind of bug. > > What you should do instead is declare your struct timespec variable > and pass a pointer to it, e.g., > struct timespec expire; <-- put that above > rtems_timespec_from_ticks( ptimer->Timer.expire, &expire ); > ^ > ^ ^ > don't forget to add spaces after ( and before ). Use & to pass the > pointer to the local stack-allocated expire struct. > > > > > - if ( now < ptimer->Timer.expire ) { > > - remaining = (uint32_t) ( ptimer->Timer.expire - now ); > > + if (ptimer->clock_type == CLOCK_MONOTONIC) { > ^ > ^ > add spaces after ( and before ) > > > + _Timecounter_Nanouptime(now); > This isn't correct. now is an uninitialized pointer. How do you > compile and run this code? I'm surprised it passes tests with this > kind of bug. > > Declare now like expire, and pass a pointer to the stack-local variable. > > > + } > > + if (ptimer- > >clock_type == CLOCK_REALTIME) { > add spaces after ( and before ) > > > + _TOD_Get(now); > This isn't correct. now is an uninitialized pointer. How do you > compile and run this code? I'm surprised it passes tests with this > kind of bug. > > > + } > > + > > + > > + if( rtems_timespec_less_than( now, expire )) { > ^ ^ > add space after if and before ) at the end. > > > + rtems_timespec_subtract(now, expire, result); > This isn't correct. now, expire, and result are uninitialized pointers. > > Declare result like now and expire, and pass a reference to the > stack-local variable. > > > } else { > > - remaining = 0; > > + result->tv_nsec=0; > > + result->tv_sec=0; > This isn't correct. result is an uninitialized pointer. > > Add spaces around =. > > > } > > + > > + (*value).it_value=*result; > why using *. instead of -> notation? > > > + value->it_interval = ptimer->timer_data.it_interval; > > + > > + _POSIX_Timer_Release(cpu, &lock_context); > > + return 0; > > +} > > + > > + > > + > > + > Don't add extra whitespaces. We don't use more than one blank line in > a row in RTEMS C Code. > > > > > - _Timespec_From_ticks( remaining, &value->it_value ); > > - value->it_interval = ptimer->timer_data.it_interval; > > > > - _POSIX_Timer_Release( cpu, &lock_context ); > > - return 0; > > - } > > > > - rtems_set_errno_and_return_minus_one( EINVAL ); > > -} > > -- > > 2.32.0 > > > > _______________________________________________ > > devel mailing list > > devel@rtems.org > > http://lists.rtems.org/mailman/listinfo/devel > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
