Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page: platform/linux-generic/odp_packet.c line 169 @@ -1352,16 +1352,14 @@ void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t flow_hash) { odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt); - pkt_hdr->flow_hash = flow_hash; - pkt_hdr->p.input_flags.flow_hash = 1; + packet_set_flow_hash(pkt_hdr, flow_hash); } void odp_packet_ts_set(odp_packet_t pkt, odp_time_t timestamp) { odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt); - pkt_hdr->timestamp = timestamp; - pkt_hdr->p.input_flags.timestamp = 1; + packet_set_ts(pkt_hdr, ×tamp);
Comment: 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_r165045138 updated_at 2018-01-31 13:54:27