If the goal is to fix the bug, Anders tiny patch does that, what else are we fixing ?
I did not look deeply enough before because I did not understand the new header layout properly, but both functions odp_version_api_str and odp_version_impl_str are part of the public API and all implementations need to implement them so they should be in include/odp/api/version.h I think. On 4 February 2015 at 05:04, Ola Liljedahl <ola.liljed...@linaro.org> wrote: > On 4 February 2015 at 10:12, Maxim Uvarov <maxim.uva...@linaro.org> wrote: > > Regarding to "static inline" even if it's not performance critical I > think > > it's reasonable to have one line function in single header. > > > > Or even switch it to: > > #define odp_version_api_str() ODP_VERSION_API_STR > Make it a proper function (does not have to be inlined) and hide the > define from the application. > I want to avoid all explicit "inlining" of platform specific > definitions. We need to keep the door open for an ODP ABI and e.g. > shared library implementations. > > > > > Maxim. > > > > > > On 02/04/2015 11:45 AM, Maxim Uvarov wrote: > >> > >> odp_version_api_str() is API function or not? If it's api it has to be > in > >> api header. If it's implementation only helper it hat to be under > >> linux-generic. > >> > >> Maxim. > >> > >> > >> > >> On 02/04/2015 01:48 AM, Anders Roxell wrote: > >>> > >>> On 2015-02-03 11:28, Mike Holmes wrote: > >>>> > >>>> On 3 February 2015 at 11:12, Maxim Uvarov <maxim.uva...@linaro.org> > >>>> wrote: > >>>> > >>>>> odp_version_api_str() has to be in API header while > >>>>> odp_version_impl_str() should be in linux-generic. > >>>>> That change fixes: > >>>>> https://bugs.linaro.org/show_bug.cgi?id=1194 > >>>>> > >>>>> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org> > >>>>> > >>>> Reviewed-by: Mike Holmes <mike.hol...@linaro.org> > >>>> > >>>> Clears the new test case patch "validation: add version string check" > >>>> which should be added as insurance against this bug in the future > before > >>>> closing the bug > >>> > >>> I think we can drop "static inline" to the version string function, > >>> because checking for a version string is not time critical right? > >>> > >>> If we assume that its not time critical the diff below + Mikes > validation > >>> version string patch got it to pass and we hide more stuff from the > >>> public doxygen clean API include/odp/api/*.h files. > >>> > >>> Cheers, > >>> Anders > >>> > >>> $ git df > >>> diff --git a/platform/linux-generic/include/odp/version.h > >>> b/platform/linux-generic/include/odp/version.h > >>> index f29320a..2c557cc 100644 > >>> --- a/platform/linux-generic/include/odp/version.h > >>> +++ b/platform/linux-generic/include/odp/version.h > >>> @@ -23,11 +23,6 @@ extern "C" { > >>> * @{ > >>> */ > >>> -static inline const char *odp_version_api_str(void) > >>> -{ > >>> - return ODP_VERSION_API_STR; > >>> -} > >>> - > >>> /** > >>> * @} > >>> */ > >>> diff --git a/platform/linux-generic/odp_impl.c > >>> b/platform/linux-generic/odp_impl.c > >>> index ca3224d..23ab12d 100644 > >>> --- a/platform/linux-generic/odp_impl.c > >>> +++ b/platform/linux-generic/odp_impl.c > >>> @@ -28,6 +28,11 @@ const char *odp_version_impl_str(void) > >>> return ODP_VERSION_IMPL_STR; > >>> } > >>> +const char *odp_version_api_str(void) > >>> +{ > >>> + return ODP_VERSION_API_STR; > >>> +} > >>> + > >>> #ifdef __cplusplus > >>> } > >>> #endif > >> > >> > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
