On 2015-04-30 10:16, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > -----Original Message----- > > From: lng-odp [mailto:[email protected]] On Behalf Of ext > > Anders Roxell > > Sent: Thursday, April 30, 2015 12:21 PM > > To: Nicolas Morey-Chaisemartin > > Cc: [email protected] > > 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.
OK, understand now. API change, but bug fix: seems reasonable > > 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) Makes sense. Cheers, Anders _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
