| Message ID | 1421237471-25485-1-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | Accepted |
| Commit | 954bb2b96ba3b2b7405cca39e798bbf9e3fdde57 |
| Headers | show |
From the timer_settime man page:
int timer_settime(timer_t timerid, int flags,
const struct itimerspec *new_value,
struct itimerspec * old_value);
On 14 January 2015 at 13:11, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
> Timerid is passed by value, not reference. Compiler cannot detect this problem
> because timer_t is defined as a void ptr on Linux.
>
> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
> Broken code worked anyway on x86-64 and ARMv7 targets but bug was detected on
> i386 target or -m32 build on x86-64 target.
>
> platform/linux-generic/odp_timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
> index ef26b02..3ba32a1 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -659,7 +659,7 @@ static void itimer_init(odp_timer_pool *tp)
> ispec.it_value.tv_sec = (time_t)sec;
> ispec.it_value.tv_nsec = (long)nsec;
>
> - if (timer_settime(&tp->timerid, 0, &ispec, NULL))
> + if (timer_settime(tp->timerid, 0, &ispec, NULL))
> ODP_ABORT("timer_settime() returned error %s\n",
> strerror(errno));
> }
> --
> 1.9.1
>
Seems like another reason to move forward with strong typing for ODP data types? On Wed, Jan 14, 2015 at 6:23 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > From the timer_settime man page: > int timer_settime(timer_t timerid, int flags, > const struct itimerspec *new_value, > struct itimerspec * old_value); > > On 14 January 2015 at 13:11, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > > Timerid is passed by value, not reference. Compiler cannot detect this > problem > > because timer_t is defined as a void ptr on Linux. > > > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > > --- > > (This document/code contribution attached is provided under the terms of > > agreement LES-LTM-21309) > > Broken code worked anyway on x86-64 and ARMv7 targets but bug was > detected on > > i386 target or -m32 build on x86-64 target. > > > > platform/linux-generic/odp_timer.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/odp_timer.c > b/platform/linux-generic/odp_timer.c > > index ef26b02..3ba32a1 100644 > > --- a/platform/linux-generic/odp_timer.c > > +++ b/platform/linux-generic/odp_timer.c > > @@ -659,7 +659,7 @@ static void itimer_init(odp_timer_pool *tp) > > ispec.it_value.tv_sec = (time_t)sec; > > ispec.it_value.tv_nsec = (long)nsec; > > > > - if (timer_settime(&tp->timerid, 0, &ispec, NULL)) > > + if (timer_settime(tp->timerid, 0, &ispec, NULL)) > > ODP_ABORT("timer_settime() returned error %s\n", > > strerror(errno)); > > } > > -- > > 1.9.1 > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 14 January 2015 at 13:44, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Seems like another reason to move forward with strong typing for ODP data > types? Yes. Too late for Linux API's though. But we should learn from mistakes. > > On Wed, Jan 14, 2015 at 6:23 AM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: >> >> From the timer_settime man page: >> int timer_settime(timer_t timerid, int flags, >> const struct itimerspec *new_value, >> struct itimerspec * old_value); >> >> On 14 January 2015 at 13:11, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >> > Timerid is passed by value, not reference. Compiler cannot detect this >> > problem >> > because timer_t is defined as a void ptr on Linux. >> > >> > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > >> >> > --- >> > (This document/code contribution attached is provided under the terms of >> > agreement LES-LTM-21309) >> > Broken code worked anyway on x86-64 and ARMv7 targets but bug was >> > detected on >> > i386 target or -m32 build on x86-64 target. >> > >> > platform/linux-generic/odp_timer.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/platform/linux-generic/odp_timer.c >> > b/platform/linux-generic/odp_timer.c >> > index ef26b02..3ba32a1 100644 >> > --- a/platform/linux-generic/odp_timer.c >> > +++ b/platform/linux-generic/odp_timer.c >> > @@ -659,7 +659,7 @@ static void itimer_init(odp_timer_pool *tp) >> > ispec.it_value.tv_sec = (time_t)sec; >> > ispec.it_value.tv_nsec = (long)nsec; >> > >> > - if (timer_settime(&tp->timerid, 0, &ispec, NULL)) >> > + if (timer_settime(tp->timerid, 0, &ispec, NULL)) >> > ODP_ABORT("timer_settime() returned error %s\n", >> > strerror(errno)); >> > } >> > -- >> > 1.9.1 >> > >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > >
Merged, Maxim. On 01/14/2015 03:11 PM, Ola Liljedahl wrote: > Timerid is passed by value, not reference. Compiler cannot detect this problem > because timer_t is defined as a void ptr on Linux. > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > --- > (This document/code contribution attached is provided under the terms of > agreement LES-LTM-21309) > Broken code worked anyway on x86-64 and ARMv7 targets but bug was detected on > i386 target or -m32 build on x86-64 target. > > platform/linux-generic/odp_timer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c > index ef26b02..3ba32a1 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -659,7 +659,7 @@ static void itimer_init(odp_timer_pool *tp) > ispec.it_value.tv_sec = (time_t)sec; > ispec.it_value.tv_nsec = (long)nsec; > > - if (timer_settime(&tp->timerid, 0, &ispec, NULL)) > + if (timer_settime(tp->timerid, 0, &ispec, NULL)) > ODP_ABORT("timer_settime() returned error %s\n", > strerror(errno)); > }
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c index ef26b02..3ba32a1 100644 --- a/platform/linux-generic/odp_timer.c +++ b/platform/linux-generic/odp_timer.c @@ -659,7 +659,7 @@ static void itimer_init(odp_timer_pool *tp) ispec.it_value.tv_sec = (time_t)sec; ispec.it_value.tv_nsec = (long)nsec; - if (timer_settime(&tp->timerid, 0, &ispec, NULL)) + if (timer_settime(tp->timerid, 0, &ispec, NULL)) ODP_ABORT("timer_settime() returned error %s\n", strerror(errno)); }
Timerid is passed by value, not reference. Compiler cannot detect this problem because timer_t is defined as a void ptr on Linux. Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- (This document/code contribution attached is provided under the terms of agreement LES-LTM-21309) Broken code worked anyway on x86-64 and ARMv7 targets but bug was detected on i386 target or -m32 build on x86-64 target. platform/linux-generic/odp_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)