| Message ID | 1426007078-28326-3-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
I think we can not touch api for stable 1.0.x. And only this patch can go to 1.0 Maxim. On 03/10/15 20:04, Bill Fischofer wrote: > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > test/validation/odp_packet.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/test/validation/odp_packet.c b/test/validation/odp_packet.c > index 8f764bf..0c1d069 100644 > --- a/test/validation/odp_packet.c > +++ b/test/validation/odp_packet.c > @@ -425,7 +425,7 @@ do { \ > odp_packet_has_##flag##_set(packet, 0); \ > CU_ASSERT(odp_packet_has_##flag(packet) == 0); \ > odp_packet_has_##flag##_set(packet, 1); \ > - CU_ASSERT(odp_packet_has_##flag(packet) == 1); \ > + CU_ASSERT(odp_packet_has_##flag(packet) != 0); \ > } while (0) > > static void packet_in_flags(void)
As Bug 1334 <https://bugs.linaro.org/show_bug.cgi?id=1334> points out. This is bringing the validation test into conformance with our own published guidelines. Any implementation that passes without this fix will still pass with it, however as currently structured the validation test will fail some implementations when it should not do that. We either need to fix the test or the guidelines to make them consistent with each other. On Tue, Mar 10, 2015 at 1:29 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > I think we can not touch api for stable 1.0.x. And only this patch can go > to 1.0 > > Maxim. > > > > > On 03/10/15 20:04, Bill Fischofer wrote: > >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> test/validation/odp_packet.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/test/validation/odp_packet.c b/test/validation/odp_packet.c >> index 8f764bf..0c1d069 100644 >> --- a/test/validation/odp_packet.c >> +++ b/test/validation/odp_packet.c >> @@ -425,7 +425,7 @@ do { \ >> odp_packet_has_##flag##_set(packet, 0); \ >> CU_ASSERT(odp_packet_has_##flag(packet) == 0); \ >> odp_packet_has_##flag##_set(packet, 1); \ >> - CU_ASSERT(odp_packet_has_##flag(packet) == 1); \ >> + CU_ASSERT(odp_packet_has_##flag(packet) != 0); \ >> } while (0) >> static void packet_in_flags(void) >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 03/10/15 21:32, Bill Fischofer wrote: > As Bug 1334 <https://bugs.linaro.org/show_bug.cgi?id=1334> points > out. This is bringing the validation test into conformance with our > own published guidelines. Any implementation that passes without this > fix will still pass with it, however as currently structured the > validation test will fail some implementations when it should not do > that. > > We either need to fix the test or the guidelines to make them > consistent with each other. > Usually the way to go here is to have 2 patches: one is for current development (1.1.X), which you did. And second patch is back port of that patch to stable tree without modifying existence API (to 1.0.X). In that case if int is returned you can fix implementation to return only 0 or 1. Of course new development patch should go first and back port patch have to have reference to original. Maxim. > On Tue, Mar 10, 2015 at 1:29 PM, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > I think we can not touch api for stable 1.0.x. And only this patch > can go to 1.0 > > Maxim. > > > > > On 03/10/15 20:04, Bill Fischofer wrote: > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > --- > test/validation/odp_packet.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/test/validation/odp_packet.c > b/test/validation/odp_packet.c > index 8f764bf..0c1d069 100644 > --- a/test/validation/odp_packet.c > +++ b/test/validation/odp_packet.c > @@ -425,7 +425,7 @@ do { \ > odp_packet_has_##flag##_set(packet, 0); \ > CU_ASSERT(odp_packet_has_##flag(packet) == 0); \ > odp_packet_has_##flag##_set(packet, 1); \ > - CU_ASSERT(odp_packet_has_##flag(packet) == 1); \ > + CU_ASSERT(odp_packet_has_##flag(packet) != 0); \ > } while (0) > static void packet_in_flags(void) > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > >
Once we decide what we want to do with this I'll be happy to repackage the patch for v2 as needed. On Tue, Mar 10, 2015 at 4:33 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/10/15 21:32, Bill Fischofer wrote: > >> As Bug 1334 <https://bugs.linaro.org/show_bug.cgi?id=1334> points out. >> This is bringing the validation test into conformance with our own >> published guidelines. Any implementation that passes without this fix will >> still pass with it, however as currently structured the validation test >> will fail some implementations when it should not do that. >> >> We either need to fix the test or the guidelines to make them consistent >> with each other. >> >> > Usually the way to go here is to have 2 patches: one is for current > development (1.1.X), which you did. And second patch is back port of that > patch to stable tree without modifying existence API (to 1.0.X). In that > case if int is returned you can fix implementation to return only 0 or 1. > Of course new development patch should go first and back port patch have to > have reference to original. > > Maxim. > > On Tue, Mar 10, 2015 at 1:29 PM, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> I think we can not touch api for stable 1.0.x. And only this patch >> can go to 1.0 >> >> Maxim. >> >> >> >> >> On 03/10/15 20:04, Bill Fischofer wrote: >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> >> --- >> test/validation/odp_packet.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/test/validation/odp_packet.c >> b/test/validation/odp_packet.c >> index 8f764bf..0c1d069 100644 >> --- a/test/validation/odp_packet.c >> +++ b/test/validation/odp_packet.c >> @@ -425,7 +425,7 @@ do { \ >> odp_packet_has_##flag##_set(packet, 0); \ >> CU_ASSERT(odp_packet_has_##flag(packet) == 0); \ >> odp_packet_has_##flag##_set(packet, 1); \ >> - CU_ASSERT(odp_packet_has_##flag(packet) == 1); \ >> + CU_ASSERT(odp_packet_has_##flag(packet) != 0); \ >> } while (0) >> static void packet_in_flags(void) >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
I would like to reword that subject from: validation: packet: use true == \!0 per ODP API Guidelines to """ validation: check packet flags not not 0 odp packet flags functions return int while it's intend to be odp_bool_t. odp_bool_t have to be checked for no 0. """ It should be better due to: 1. "== \!0" probably have to be "=! 0" 2. It's better not add symbols which you need escape with \. I.e. short subject has to be easy search-able. Maxim. On 03/10/15 20:04, Bill Fischofer wrote: > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > test/validation/odp_packet.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/test/validation/odp_packet.c b/test/validation/odp_packet.c > index 8f764bf..0c1d069 100644 > --- a/test/validation/odp_packet.c > +++ b/test/validation/odp_packet.c > @@ -425,7 +425,7 @@ do { \ > odp_packet_has_##flag##_set(packet, 0); \ > CU_ASSERT(odp_packet_has_##flag(packet) == 0); \ > odp_packet_has_##flag##_set(packet, 1); \ > - CU_ASSERT(odp_packet_has_##flag(packet) == 1); \ > + CU_ASSERT(odp_packet_has_##flag(packet) != 0); \ > } while (0) > > static void packet_in_flags(void)
diff --git a/test/validation/odp_packet.c b/test/validation/odp_packet.c index 8f764bf..0c1d069 100644 --- a/test/validation/odp_packet.c +++ b/test/validation/odp_packet.c @@ -425,7 +425,7 @@ do { \ odp_packet_has_##flag##_set(packet, 0); \ CU_ASSERT(odp_packet_has_##flag(packet) == 0); \ odp_packet_has_##flag##_set(packet, 1); \ - CU_ASSERT(odp_packet_has_##flag(packet) == 1); \ + CU_ASSERT(odp_packet_has_##flag(packet) != 0); \ } while (0) static void packet_in_flags(void)
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- test/validation/odp_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)