Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-generic/odp_packet.c
line 28
@@ -49,15 +49,14 @@ const _odp_packet_inline_offset_t ODP_ALIGNED_CACHE
_odp_packet_inline = {
#include <odp/visibility_end.h>
-static inline odp_buffer_t buffer_handle(odp_packet_hdr_t *pkt_hdr)
-{
- return (odp_buffer_t)pkt_hdr;
-}
-
-static inline odp_packet_hdr_t *buf_to_packet_hdr(odp_buffer_t buf)
-{
- return (odp_packet_hdr_t *)buf_hdl_to_hdr(buf);
-}
+/* Check that invalid values are the same. Some versions of Clang have trouble
+ * with the strong type casting, and complain that these invalid values are not
+ * integral constants. */
+#ifndef __clang__
+ODP_STATIC_ASSERT(ODP_PACKET_INVALID == 0, "Packet invalid not 0");
+ODP_STATIC_ASSERT(ODP_BUFFER_INVALID == 0, "Buffer invalid not 0");
+ODP_STATIC_ASSERT(ODP_EVENT_INVALID == 0, "Event invalid not 0");
Comment:
Maybe `(uintptr_t)(handle) == (uintptr_t)0` would do the trick?
> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> Very strange. I'm Ok with this workaround, but a comment about clang version
> might be appropriate. Don't know if it's worth doing a clang version check.
> We do things like that in the `ODP_STATIC_ASSERT()` macro itself for similar
> reasons.
>> Petri Savolainen(psavol) wrote:
>> odp_packet.c:52:19: error: static_assert expression is not an integral
>> constant
>> expression
>> ODP_STATIC_ASSERT(_odp_typeval(ODP_BUFFER_INVALID) == 0, "Buffer inval not
>> 0");
>> ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../../platform/linux-generic/include/odp/api/plat/strong_types.h:32:30:
>> note:
>> expanded from macro '_odp_typeval'
>> #define _odp_typeval(handle) ((uint32_t)(uintptr_t)(handle))
>> ^
>> ../../include/odp/api/abi-default/debug.h:26:53: note: expanded from macro
>> 'ODP_STATIC_ASSERT'
>> #define ODP_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
>> ^~~~
>> odp_packet.c:52:19: note: cast that performs the conversions of a
>> reinterpret_cast is not allowed in a constant expression
>> ../../platform/linux-generic/include/odp/api/plat/strong_types.h:32:41:
>> note:
>> expanded from macro '_odp_typeval'
>> #define _odp_typeval(handle) ((uint32_t)(uintptr_t)(handle))
>>> Petri Savolainen(psavol) wrote:
>>> static_assert() seems special. The clang version in Travis does not accept
>>> type casts with static_assert(). The cast confuses it to think that
>>> (uintptr_t)0 == 0 is not an integral constant.
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> `_odp_typeval()` is defined as:
>>>> ```
>>>> #define _odp_typeval(handle) ((uint32_t)(uintptr_t)(handle))
>>>> ```
>>>> It's used elsewhere in the ODP code and clang doesn't have any problem
>>>> with it. I've verified this works for clang 4.2.1. Since it works
>>>> elsewhere on Travis I'm not sure why it wouldn't in this instance.
>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> I'm OK with just a modified commit name. I agree we needn't be too
>>>>> granular about these things.
>>>>>> Petri Savolainen(psavol) wrote:
>>>>>> I could rename the patch to:
>>>>>> "linux-gen: packet: use inlined flow hash and ts set"
>>>>>>
>>>>>> ... if necessary. The comment says already that also ts set is modified
>>>>>> the same way.
>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>> I did try several combinations of type casts of invalid values, also
>>>>>>> cast to uintptr_t. The problem is that gcc and clang (clang version
>>>>>>> 3.8.0-2ubuntu4) of my machine accepted all of those casts, but the
>>>>>>> clang version in Travis (clang version 3.8.0-2ubuntu3~trusty5) does not
>>>>>>> accept anything.
>>>>>>>
>>>>>>> Maybe it's a bug in the particular version of clang. Anyway, addition
>>>>>>> of this check even only on gcc side is improvement against what we
>>>>>>> currently have (no check). Packet invalid is currently 0xfffffff in ABI
>>>>>>> spec, but event/buffer invalid are 0.
>>>>>>>
>>>>>>> odp_packet.c:55:19: error: static_assert expression is not an integral
>>>>>>> constant
>>>>>>> expression
>>>>>>> ODP_STATIC_ASSERT(((uintptr_t)ODP_PACKET_INVALID) == 0, "Packet invalid
>>>>>>> not 0");
>>>>>>> ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>>> ../../include/odp/api/abi-default/debug.h:26:53: note: expanded from
>>>>>>> macro
>>>>>>> 'ODP_STATIC_ASSERT'
>>>>>>> #define ODP_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
>>>>>>> ^~~~
>>>>>>> odp_packet.c:55:20: note: cast that performs the conversions of a
>>>>>>> reinterpret_cast is not allowed in a constant expression
>>>>>>> ODP_STATIC_ASSERT(((uintptr_t)ODP_PACKET_INVALID) == 0, "Packet invalid
>>>>>>> not 0");
>>>>>>> ^
>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> Since you want to breech the strong type you need to use the internal
>>>>>>>> `odp_typeval()` macro here.
>>>>>>>> ```
>>>>>>>> ODP_STATIC_ASSERT(_odp_typeval(ODP_EVENT_INVALID) == 0, "Event invalid
>>>>>>>> not 0");
>>>>>>>> ```
>>>>>>>> etc. I've verified that this works fine for clang, so no need for the
>>>>>>>> conditional compilation.
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> Should this be in its own commit? Different function.
https://github.com/Linaro/odp/pull/437#discussion_r165111717
updated_at 2018-01-31 16:38:56
