| Message ID | 1428321241-1552-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Accepted |
| Commit | 46b3dd9df9f0a2abca7095dd4d4b25a38c6e8271 |
| Headers | show |
On Mon, Apr 6, 2015 at 6:54 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > Fixes CID 89196 > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > test/validation/classification/odp_classification_tests.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/test/validation/classification/odp_classification_tests.c > b/test/validation/classification/odp_classification_tests.c > index 0530f99..1bf080f 100644 > --- a/test/validation/classification/odp_classification_tests.c > +++ b/test/validation/classification/odp_classification_tests.c > @@ -126,6 +126,7 @@ static int cls_pkt_set_seq(odp_packet_t pkt) > static uint32_t seq; > cls_test_packet_t data; > uint32_t offset; > + int status; > > data.magic = DATA_MAGIC; > data.seq = ++seq; > @@ -133,10 +134,10 @@ static int cls_pkt_set_seq(odp_packet_t pkt) > offset = odp_packet_l4_offset(pkt); > CU_ASSERT_FATAL(offset != 0); > > - odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, > - sizeof(data), &data); > + status = odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, > + sizeof(data), &data); > > Wouldn't it be simpler to say: return odp_packet_copydata_in(...); ? > - return 0; > + return status; > } > > static uint32_t cls_pkt_get_seq(odp_packet_t pkt) > -- > 2.1.0 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 6 April 2015 at 08:00, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Mon, Apr 6, 2015 at 6:54 AM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> Fixes CID 89196 >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> --- >> test/validation/classification/odp_classification_tests.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/test/validation/classification/odp_classification_tests.c >> b/test/validation/classification/odp_classification_tests.c >> index 0530f99..1bf080f 100644 >> --- a/test/validation/classification/odp_classification_tests.c >> +++ b/test/validation/classification/odp_classification_tests.c >> @@ -126,6 +126,7 @@ static int cls_pkt_set_seq(odp_packet_t pkt) >> static uint32_t seq; >> cls_test_packet_t data; >> uint32_t offset; >> + int status; >> >> data.magic = DATA_MAGIC; >> data.seq = ++seq; >> @@ -133,10 +134,10 @@ static int cls_pkt_set_seq(odp_packet_t pkt) >> offset = odp_packet_l4_offset(pkt); >> CU_ASSERT_FATAL(offset != 0); >> >> - odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, >> - sizeof(data), &data); >> + status = odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, >> + sizeof(data), &data); >> >> > Wouldn't it be simpler to say: > > return odp_packet_copydata_in(...); ? > I find it easier to read a return which is not also a function call. I also find it easier to single step in a debugger with this because I can stop in the function after the call more clearly. > > >> - return 0; >> + return status; >> } >> >> static uint32_t cls_pkt_get_seq(odp_packet_t pkt) >> -- >> 2.1.0 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
No problem with that as a reason, so: Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> On Mon, Apr 6, 2015 at 7:18 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 6 April 2015 at 08:00, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> >> >> On Mon, Apr 6, 2015 at 6:54 AM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> Fixes CID 89196 >>> >>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>> --- >>> test/validation/classification/odp_classification_tests.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/test/validation/classification/odp_classification_tests.c >>> b/test/validation/classification/odp_classification_tests.c >>> index 0530f99..1bf080f 100644 >>> --- a/test/validation/classification/odp_classification_tests.c >>> +++ b/test/validation/classification/odp_classification_tests.c >>> @@ -126,6 +126,7 @@ static int cls_pkt_set_seq(odp_packet_t pkt) >>> static uint32_t seq; >>> cls_test_packet_t data; >>> uint32_t offset; >>> + int status; >>> >>> data.magic = DATA_MAGIC; >>> data.seq = ++seq; >>> @@ -133,10 +134,10 @@ static int cls_pkt_set_seq(odp_packet_t pkt) >>> offset = odp_packet_l4_offset(pkt); >>> CU_ASSERT_FATAL(offset != 0); >>> >>> - odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, >>> - sizeof(data), &data); >>> + status = odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, >>> + sizeof(data), &data); >>> >>> >> Wouldn't it be simpler to say: >> >> return odp_packet_copydata_in(...); ? >> > > I find it easier to read a return which is not also a function call. > I also find it easier to single step in a debugger with this because I can > stop in the function after the call more clearly. > > >> >> >>> - return 0; >>> + return status; >>> } >>> >>> static uint32_t cls_pkt_get_seq(odp_packet_t pkt) >>> -- >>> 2.1.0 >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > > >
Merged, Maxim. On 04/06/15 15:21, Bill Fischofer wrote: > No problem with that as a reason, so: > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > > On Mon, Apr 6, 2015 at 7:18 AM, Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > > > On 6 April 2015 at 08:00, Bill Fischofer > <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote: > > > > On Mon, Apr 6, 2015 at 6:54 AM, Mike Holmes > <mike.holmes@linaro.org <mailto:mike.holmes@linaro.org>> wrote: > > Fixes CID 89196 > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > --- > test/validation/classification/odp_classification_tests.c > | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git > a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c > index 0530f99..1bf080f 100644 > --- > a/test/validation/classification/odp_classification_tests.c > +++ > b/test/validation/classification/odp_classification_tests.c > @@ -126,6 +126,7 @@ static int > cls_pkt_set_seq(odp_packet_t pkt) > static uint32_t seq; > cls_test_packet_t data; > uint32_t offset; > + int status; > > data.magic = DATA_MAGIC; > data.seq = ++seq; > @@ -133,10 +134,10 @@ static int > cls_pkt_set_seq(odp_packet_t pkt) > offset = odp_packet_l4_offset(pkt); > CU_ASSERT_FATAL(offset != 0); > > - odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, > - sizeof(data), &data); > + status = odp_packet_copydata_in(pkt, offset + > ODPH_UDPHDR_LEN, > + sizeof(data), &data); > > > Wouldn't it be simpler to say: > > return odp_packet_copydata_in(...); ? > > > I find it easier to read a return which is not also a function call. > I also find it easier to single step in a debugger with this > because I can stop in the function after the call more clearly. > > - return 0; > + return status; > } > > static uint32_t cls_pkt_get_seq(odp_packet_t pkt) > -- > 2.1.0 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for > ARM SoCs > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c index 0530f99..1bf080f 100644 --- a/test/validation/classification/odp_classification_tests.c +++ b/test/validation/classification/odp_classification_tests.c @@ -126,6 +126,7 @@ static int cls_pkt_set_seq(odp_packet_t pkt) static uint32_t seq; cls_test_packet_t data; uint32_t offset; + int status; data.magic = DATA_MAGIC; data.seq = ++seq; @@ -133,10 +134,10 @@ static int cls_pkt_set_seq(odp_packet_t pkt) offset = odp_packet_l4_offset(pkt); CU_ASSERT_FATAL(offset != 0); - odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, - sizeof(data), &data); + status = odp_packet_copydata_in(pkt, offset + ODPH_UDPHDR_LEN, + sizeof(data), &data); - return 0; + return status; } static uint32_t cls_pkt_get_seq(odp_packet_t pkt)
Fixes CID 89196 Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- test/validation/classification/odp_classification_tests.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)