| Message ID | 1450458040-16391-1-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | Accepted |
| Commit | 40969bbc95d84fbec78ef083a0015884fdca8e9a |
| Headers | show |
On Fri, Dec 18, 2015 at 05:00:40PM +0000, Zoltan Kiss wrote: > The callers of this function expect TEST_SEQ_INVALID or the sequence > number. Although it is defined as (uint32_t)~0, which normally yields to > -1, it's cleaner to use the same macro. > Similar to that, it's unlikely that seq reaches UINT32_MAX, the function > should check for it. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> Reviewed-by: Stuart Haslam <stuart.haslam@linaro.org> > --- > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index 4de3ff8..ff6ece9 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -142,7 +142,7 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt) > pkt_tail_t tail; > > if (pkt == ODP_PACKET_INVALID) > - return -1; > + return TEST_SEQ_INVALID; A patch I sent earlier today includes this hunk: http://patches.opendataplane.org/patch/4236/ > > off = odp_packet_l4_offset(pkt); > if (off == ODP_PACKET_OFFSET_INVALID) > @@ -160,8 +160,10 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt) > if (odp_packet_copydata_out(pkt, off, sizeof(tail), &tail) != 0) > return TEST_SEQ_INVALID; > > - if (tail.magic == TEST_SEQ_MAGIC) > + if (tail.magic == TEST_SEQ_MAGIC) { > seq = head.seq; > + CU_ASSERT(seq != TEST_SEQ_INVALID); > + } But not this one.
Merged, Maxim. On 12/18/2015 20:06, Stuart Haslam wrote: > On Fri, Dec 18, 2015 at 05:00:40PM +0000, Zoltan Kiss wrote: >> The callers of this function expect TEST_SEQ_INVALID or the sequence >> number. Although it is defined as (uint32_t)~0, which normally yields to >> -1, it's cleaner to use the same macro. >> Similar to that, it's unlikely that seq reaches UINT32_MAX, the function >> should check for it. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > Reviewed-by: Stuart Haslam <stuart.haslam@linaro.org> > >> --- >> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c >> index 4de3ff8..ff6ece9 100644 >> --- a/test/validation/pktio/pktio.c >> +++ b/test/validation/pktio/pktio.c >> @@ -142,7 +142,7 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt) >> pkt_tail_t tail; >> >> if (pkt == ODP_PACKET_INVALID) >> - return -1; >> + return TEST_SEQ_INVALID; > A patch I sent earlier today includes this hunk: > > http://patches.opendataplane.org/patch/4236/ > >> >> off = odp_packet_l4_offset(pkt); >> if (off == ODP_PACKET_OFFSET_INVALID) >> @@ -160,8 +160,10 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt) >> if (odp_packet_copydata_out(pkt, off, sizeof(tail), &tail) != 0) >> return TEST_SEQ_INVALID; >> >> - if (tail.magic == TEST_SEQ_MAGIC) >> + if (tail.magic == TEST_SEQ_MAGIC) { >> seq = head.seq; >> + CU_ASSERT(seq != TEST_SEQ_INVALID); >> + } > But not this one. > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 4de3ff8..ff6ece9 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -142,7 +142,7 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt) pkt_tail_t tail; if (pkt == ODP_PACKET_INVALID) - return -1; + return TEST_SEQ_INVALID; off = odp_packet_l4_offset(pkt); if (off == ODP_PACKET_OFFSET_INVALID) @@ -160,8 +160,10 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt) if (odp_packet_copydata_out(pkt, off, sizeof(tail), &tail) != 0) return TEST_SEQ_INVALID; - if (tail.magic == TEST_SEQ_MAGIC) + if (tail.magic == TEST_SEQ_MAGIC) { seq = head.seq; + CU_ASSERT(seq != TEST_SEQ_INVALID); + } } return seq;
The callers of this function expect TEST_SEQ_INVALID or the sequence number. Although it is defined as (uint32_t)~0, which normally yields to -1, it's cleaner to use the same macro. Similar to that, it's unlikely that seq reaches UINT32_MAX, the function should check for it. Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> ---