On 02/04/2015 05:43 PM, Mike Holmes wrote:
If the goal is to fix the bug, Anders tiny patch does that, what else
are we fixing ?
it's better to take a look at v4 of my patch.
odp_impc. has defines copy pasted from header:
-#ifndef ODP_IMPL_H_
-#define ODP_IMPL_H_
+ implementation of odp_version_api_str() should be there and it should
not be inline.
So v4 patch is for that.
Maxim.
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
<mailto:ola.liljed...@linaro.org>> wrote:
On 4 February 2015 at 10:12, Maxim Uvarov <maxim.uva...@linaro.org
<mailto: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 <mailto: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
<mailto:maxim.uva...@linaro.org>>
>>>>>
>>>> Reviewed-by: Mike Holmes <mike.hol...@linaro.org
<mailto: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 <mailto: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
