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