> -----Original Message-----
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext
> Anders Roxell
> Sent: Thursday, April 30, 2015 12:21 PM
> To: Nicolas Morey-Chaisemartin
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH] api: time: force time defines as ULL to
> avoid computation overflows on 32bits systems
> 
> On 2015-04-30 09:42, Nicolas Morey-Chaisemartin wrote:
> >
> >
> > On 04/29/2015 08:43 PM, Mike Holmes wrote:
> > > But this is in the public API so it may be used in an application and
> it
> > > could now change the size of a structure in memory.
> >
> > Maybe I'm missing something but I don't see how. This is only a literal
> value. The time type used by the API is already on 64b.
> 
> Just to clarify what I meat.
> The test that failed was an application seen from the ODP perspective.
> Through this change in ODP we change how the application compiles and
> works, so this change passes the interface between the ODP and the
> application. And that's why I think this is an API change.
> 
> NOTE: This change wont affect all platforms (those defining ULONG as 64
> bits wont be affected), but it will affect other applications
> on other platforms.
> 
> Cheers,
> Anders
> 


We can think it also as a bug fix. Application expects this to work...

uint64_t time = 10*ODP_TIME_SEC;

... but without the change, application needs to do this to be portable for 32 
bit ...

uint64_t time = 10*(uint64_t)ODP_TIME_SEC;

... which was not intention of the API.


It's on the border, but I think that API bug corrections like this are OK for 
the master branch. ODP_VERSION_API_MINOR is there for this kind of fixes.

We could actually make it still more explicit that time is a uint64_t type 
(this time through api-next).

#define ODP_TIME_SEC  ((uint64_t)1000000000)

-Petri


_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to