> - remaining = 0; > + result->tv_nsec=0; > + result->tv_sec=0; This isn't correct. result is an uninitialized pointer.
So then how do I set the timespec's value to 0? I have changed the timespec variables i have declared from pointers to regular variables. in the original code the result ( used to be the remaining time) should be 0. > + _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. Since the now timespec is defined as struct timesepc now TOD_GET used like this : _TOD_GET(now) I think the code will now work as intended as it now has a reference. > > - 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. Can you explain where the whilespace is? Sometimes i have issues with trailing whitespace as sometimes when i run format patch it's different from the file that I'm working on. > + > + > + > + Don't add extra whitespaces. We don't use more than one blank line in a row in RTEMS C Again I may have to change it manually within the patch file. I may talk with people on discord for help to avoid flooding the mailing list. Also how i solved the nesting was the completely delete the repository and then i would rebuilt rtems. After making a emailed patch how do i do another one without running into the same problem? On Fri, 23 Jul 2021 at 22:15, Joel Sherrill <j...@rtems.org> wrote: > 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