Re: [PATCH] closes #3889
Please work on fixing the commit message for your patch locally as
discussed before, and use git-format-patch to look at how your commit
looks like in a file before you send it to the mailing list.
Use -v2 with git-format-patch to prepare the patch for submission, to
provide the email subject line with a version 2 tag.
On Tue, Jul 27, 2021 at 6:25 PM Zacchaeus Leung
wrote:
>
> adds
>
> ---
> cpukit/include/rtems/posix/timer.h| 1 +
> cpukit/posix/src/psxtimercreate.c | 5 +-
> cpukit/posix/src/timergettime.c | 59 +--
> testsuites/psxtests/psxtimer02/psxtimer.c | 26 ++
> 4 files changed, 63 insertions(+), 28 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..5ffdf429b4 100644
> --- a/cpukit/posix/src/psxtimercreate.c
> +++ b/cpukit/posix/src/psxtimercreate.c
> @@ -40,7 +40,7 @@ int timer_create(
> {
>POSIX_Timer_Control *ptimer;
>
> - if ( clock_id != CLOCK_REALTIME )
> + if ( clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC )
> rtems_set_errno_and_return_minus_one( EINVAL );
>
>if ( !timerid )
> @@ -91,7 +91,8 @@ 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( >Timer, _Per_CPU_Get_snapshot() );
>_Watchdog_Initialize( >Timer, _POSIX_Timer_TSR );
>_Objects_Open_u32(&_POSIX_Timer_Information, >Object, 0);
> diff --git a/cpukit/posix/src/timergettime.c b/cpukit/posix/src/timergettime.c
> index ee2a566f0e..2576d7ba95 100644
> --- a/cpukit/posix/src/timergettime.c
> +++ b/cpukit/posix/src/timergettime.c
> @@ -13,8 +13,6 @@
> * On-Line Applications Research Corporation (OAR).
> *
> * The license and distribution terms for this file may be
> - * found in the file LICENSE in this distribution or at
> - * http://www.rtems.org/license/LICENSE.
This should not be removed
> */
>
> #ifdef HAVE_CONFIG_H
> @@ -28,6 +26,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * - When a timer is initialized, the value of the time in
> @@ -35,39 +34,47 @@
> * - When this function is called, it returns the difference
> *between the current time and the initialization time.
> */
> -
avoid making random whitespace changes
> int timer_gettime(
>timer_ttimerid,
>struct itimerspec *value
> )
> {
>POSIX_Timer_Control *ptimer;
> - ISR_lock_Context lock_context;
> - uint64_t now;
> - uint32_t remaining;
> + ISR_lock_Context lock_context;
> + Per_CPU_Control *cpu;
> + struct timespec now;
> + struct timespec expire;
> + struct timespec result;
>
> - if ( !value )
> + if ( !value )
I don't see what is changing here?
> rtems_set_errno_and_return_minus_one( EINVAL );
>
> - ptimer = _POSIX_Timer_Get( timerid, _context );
> - if ( ptimer != NULL ) {
> -Per_CPU_Control *cpu;
> -
> -cpu = _POSIX_Timer_Acquire_critical( ptimer, _context );
> -now = cpu->Watchdog.ticks;
> -
> -if ( now < ptimer->Timer.expire ) {
> - remaining = (uint32_t) ( ptimer->Timer.expire - now );
> -} else {
> - remaining = 0;
> -}
> + ptimer = _POSIX_Timer_Get(timerid, _context);
don't change the style within the existing code. Focus your changes on
functional modifications.
> + if ( ptimer == NULL ) {
> +rtems_set_errno_and_return_minus_one( EINVAL );
> + }
>
> -_Timespec_From_ticks( remaining, >it_value );
> -value->it_interval = ptimer->timer_data.it_interval;
> + cpu = _POSIX_Timer_Acquire_critical( ptimer, _context ) ;
> + rtems_timespec_from_ticks( ptimer->Timer.expire, ) ;
>
> -_POSIX_Timer_Release( cpu, _context );
> -return 0;
> + if (ptimer->clock_type == CLOCK_MONOTONIC) {
> + _Timecounter_Nanouptime( );
> + }
> + if (ptimer->clock_type == CLOCK_REALTIME) {
> +_TOD_Get( );
>}
>
> - rtems_set_errno_and_return_minus_one( EINVAL );
> -}
> +
> + if( rtems_timespec_less_than( , ) ) {
add space after if before (.
> + rtems_timespec_subtract(, , );
> +} else {
> + result.tv_nsec = 0;
> + result.tv_sec = 0;
> +}
> +
> + value->it_value=result;
> + value->it_interval = ptimer->timer_data.it_interval;
> +
> +
Re: [PATCH] closes #3889
On Sat, Jul 24, 2021 at 6:10 PM zack leung wrote:
>
> > - 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.
>
Then you can use
result.tv_nsec = 0;
result.tv_sec = 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.
>
It should be _TOD_Get()? Otherwise, you pass a copy of the struct,
which then gets treated like it is a pointer (and that would be wrong
also).
>
> >
> > - ptimer = _POSIX_Timer_Get( timerid, _context );
> > - if ( ptimer != NULL ) {
> > -Per_CPU_Control *cpu;
> > + ptimer = _POSIX_Timer_Get(timerid, _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.
>
You remove single space characters within the parentheses of the
function call, "( timerid" not "(timerid")
> > +
> > +
> > +
> > +
> 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.
>
It will be much better for you to learn how to use git, than try to
manually change path files.
>
> 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?
>
Use git-rebase with interactive mode. You should spend some time
learning how to use git.
>
>
>
>
>
> On Fri, 23 Jul 2021 at 22:15, Joel Sherrill 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 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
>> > 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( >Timer, _Per_CPU_Get_snapshot() );
>> > >_Watchdog_Initialize( >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
>> > > #include
>> > > #include
>> > > +#include
>> > >
>> > > /*
>> > > * - When
Re: [PATCH] closes #3889
> - 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, _context );
> - if ( ptimer != NULL ) {
> -Per_CPU_Control *cpu;
> + ptimer = _POSIX_Timer_Get(timerid, _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 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 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
> > 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( >Timer, _Per_CPU_Get_snapshot() );
> > >_Watchdog_Initialize( >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
> > > #include
> > > #include
> > > +#include
> > >
> > > /*
> > > * - 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_ttimerid,
> > >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;
> > > +
Re: [PATCH] closes #3889
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 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
> 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( >Timer, _Per_CPU_Get_snapshot() );
> >_Watchdog_Initialize( >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
> > #include
> > #include
> > +#include
> >
> > /*
> > * - 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_ttimerid,
> >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, _context );
> > - if ( ptimer != NULL ) {
> > -Per_CPU_Control *cpu;
> > + ptimer = _POSIX_Timer_Get(timerid, _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, _context );
> > -now = cpu->Watchdog.ticks;
> > + cpu = _POSIX_Timer_Acquire_critical(ptimer, _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, );
> ^
> ^ ^
> 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) (
Re: [PATCH] closes #3889
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
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( >Timer, _Per_CPU_Get_snapshot() );
>_Watchdog_Initialize( >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
> #include
> #include
> +#include
>
> /*
> * - 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_ttimerid,
>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, _context );
> - if ( ptimer != NULL ) {
> -Per_CPU_Control *cpu;
> + ptimer = _POSIX_Timer_Get(timerid, _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, _context );
> -now = cpu->Watchdog.ticks;
> + cpu = _POSIX_Timer_Acquire_critical(ptimer, _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, );
^
^ ^
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
