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:
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_r165047601
updated_at 2018-01-31 13:54:27
