Petri Savolainen(psavol) 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:
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_r165060781
updated_at 2018-01-31 14:05:40
