[dpdk-stable] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[dpdk-stable] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Dmitry Kozlyuk]

Use "%u" and a cast as in other places when port ID is formatted.
This fixes -Wformat warning with clang 10.0.0 on Windows.

Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 examples/rxtx_callbacks/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
index b57b2fc6bc..9574b6ea0d 100644
--- a/examples/rxtx_callbacks/main.c
+++ b/examples/rxtx_callbacks/main.c
@@ -329,8 +329,8 @@ main(int argc, char *argv[])
 	/* initialize all ports */
 	RTE_ETH_FOREACH_DEV(portid)
 		if (port_init(portid, mbuf_pool) != 0)
-			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
-					portid);
+			rte_exit(EXIT_FAILURE, "Cannot init port %u\n",
+					(unsigned int)portid);
 
 	if (rte_lcore_count() > 1)
 		printf("\nWARNING: Too much enabled lcores - "
-- 
2.29.3

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Tyler Retzlaff]

On Sun, May 02, 2021 at 05:56:56AM +0300, Dmitry Kozlyuk wrote:
> Use "%u" and a cast as in other places when port ID is formatted.
> This fixes -Wformat warning with clang 10.0.0 on Windows.
> 
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  examples/rxtx_callbacks/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
> index b57b2fc6bc..9574b6ea0d 100644
> --- a/examples/rxtx_callbacks/main.c
> +++ b/examples/rxtx_callbacks/main.c
> @@ -329,8 +329,8 @@ main(int argc, char *argv[])
>  	/* initialize all ports */
>  	RTE_ETH_FOREACH_DEV(portid)
>  		if (port_init(portid, mbuf_pool) != 0)
> -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> -					portid);

how come not just `% " PRIu16 "\n"' ?

what was the -Wformat clang on windows complaint?

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Dmitry Kozlyuk]

2021-05-03 17:11 (UTC-0700), Tyler Retzlaff:
> On Sun, May 02, 2021 at 05:56:56AM +0300, Dmitry Kozlyuk wrote:
> > Use "%u" and a cast as in other places when port ID is formatted.
> > This fixes -Wformat warning with clang 10.0.0 on Windows.
> > 
> > Fixes: f8244c6399d9 ("ethdev: increase port id range")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > ---
> >  examples/rxtx_callbacks/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
> > index b57b2fc6bc..9574b6ea0d 100644
> > --- a/examples/rxtx_callbacks/main.c
> > +++ b/examples/rxtx_callbacks/main.c
> > @@ -329,8 +329,8 @@ main(int argc, char *argv[])
> >  	/* initialize all ports */
> >  	RTE_ETH_FOREACH_DEV(portid)
> >  		if (port_init(portid, mbuf_pool) != 0)
> > -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> > -					portid);  
> 
> how come not just `% " PRIu16 "\n"' ?
> 
> what was the -Wformat clang on windows complaint?

PRIx16 would work, but I noticed that in other places where port ID is
printed, the pattern above is used. IMO uniform approach is better.

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Tyler Retzlaff]

On Tue, May 04, 2021 at 09:48:22AM +0300, Dmitry Kozlyuk wrote:
> 2021-05-03 17:11 (UTC-0700), Tyler Retzlaff:
> > On Sun, May 02, 2021 at 05:56:56AM +0300, Dmitry Kozlyuk wrote:
> > > Use "%u" and a cast as in other places when port ID is formatted.
> > > This fixes -Wformat warning with clang 10.0.0 on Windows.
> > > 
> > > Fixes: f8244c6399d9 ("ethdev: increase port id range")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > ---
> > >  examples/rxtx_callbacks/main.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
> > > index b57b2fc6bc..9574b6ea0d 100644
> > > --- a/examples/rxtx_callbacks/main.c
> > > +++ b/examples/rxtx_callbacks/main.c
> > > @@ -329,8 +329,8 @@ main(int argc, char *argv[])
> > >  	/* initialize all ports */
> > >  	RTE_ETH_FOREACH_DEV(portid)
> > >  		if (port_init(portid, mbuf_pool) != 0)
> > > -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> > > -					portid);  
> > 
> > how come not just `% " PRIu16 "\n"' ?
> > 
> > what was the -Wformat clang on windows complaint?
> 
> PRIx16 would work, but I noticed that in other places where port ID is
> printed, the pattern above is used. IMO uniform approach is better.

ah, consistency. yes i'll have some of that. maybe one day in the future
we can change them all in one shot, but not today.

Acked-By: Tyler Retzlaff <roretzla@linux.microsoft.com>

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Thomas Monjalon]

05/05/2021 18:00, Tyler Retzlaff:
> On Tue, May 04, 2021 at 09:48:22AM +0300, Dmitry Kozlyuk wrote:
> > 2021-05-03 17:11 (UTC-0700), Tyler Retzlaff:
> > > On Sun, May 02, 2021 at 05:56:56AM +0300, Dmitry Kozlyuk wrote:
> > > > Use "%u" and a cast as in other places when port ID is formatted.
> > > > This fixes -Wformat warning with clang 10.0.0 on Windows.
> > > > 
> > > > Fixes: f8244c6399d9 ("ethdev: increase port id range")
> > > > Cc: stable@dpdk.org
> > > > 
> > > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
[...]
> > > > -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> > > > -					portid);  
> > > 
> > > how come not just `% " PRIu16 "\n"' ?
> > > 
> > > what was the -Wformat clang on windows complaint?
> > 
> > PRIx16 would work, but I noticed that in other places where port ID is
> > printed, the pattern above is used. IMO uniform approach is better.
> 
> ah, consistency. yes i'll have some of that. maybe one day in the future
> we can change them all in one shot, but not today.
> 
> Acked-By: Tyler Retzlaff <roretzla@linux.microsoft.com>

I think PRIu16 is more appropriate.
Consistency doesn't matter if an approach is better.

[dpdk-stable] [PATCH v2] examples/rxtx_callbacks: fix port ID format specifier

[Dmitry Kozlyuk]

This fixes -Wformat warning with clang 10.0.0 on Windows.

Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
v2: use PRIx16 instead of %u and a cast (Tyler, Thomas).

 examples/rxtx_callbacks/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
index b57b2fc6bc..ecc2da2d8c 100644
--- a/examples/rxtx_callbacks/main.c
+++ b/examples/rxtx_callbacks/main.c
@@ -329,7 +329,7 @@ main(int argc, char *argv[])
 	/* initialize all ports */
 	RTE_ETH_FOREACH_DEV(portid)
 		if (port_init(portid, mbuf_pool) != 0)
-			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
+			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIx16"\n",
 					portid);
 
 	if (rte_lcore_count() > 1)
-- 
2.29.3

[dpdk-stable] [PATCH v3] examples/rxtx_callbacks: fix port ID format specifier

[Dmitry Kozlyuk]

This fixes -Wformat warning with clang 10.0.0 on Windows.

Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
v3: PRIx16 -> PRIu16
v2: u% and a cast -> PRIx16 (Tyler, Thomas)
 examples/rxtx_callbacks/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
index b57b2fc6bc..192521c3c6 100644
--- a/examples/rxtx_callbacks/main.c
+++ b/examples/rxtx_callbacks/main.c
@@ -329,7 +329,7 @@ main(int argc, char *argv[])
 	/* initialize all ports */
 	RTE_ETH_FOREACH_DEV(portid)
 		if (port_init(portid, mbuf_pool) != 0)
-			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
+			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu16"\n",
 					portid);
 
 	if (rte_lcore_count() > 1)
-- 
2.29.3

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Tyler Retzlaff]

On Wed, May 05, 2021 at 11:39:23PM +0200, Thomas Monjalon wrote:
> 05/05/2021 18:00, Tyler Retzlaff:
> > On Tue, May 04, 2021 at 09:48:22AM +0300, Dmitry Kozlyuk wrote:
> > > > 
> > > > what was the -Wformat clang on windows complaint?
> > > 
> > > PRIx16 would work, but I noticed that in other places where port ID is
> > > printed, the pattern above is used. IMO uniform approach is better.
> > 
> > ah, consistency. yes i'll have some of that. maybe one day in the future
> > we can change them all in one shot, but not today.
> > 
> > Acked-By: Tyler Retzlaff <roretzla@linux.microsoft.com>
> 
> I think PRIu16 is more appropriate.

works for me.

> Consistency doesn't matter if an approach is better.

well, there is some value in having consistency even if it is something
sub-optimal if we intend to do a mechanical change of all instances in
the future.

as a side question, what is the projects stance on getting more warnings
clean? there are a few not enabled that i'd really like to see e.g.
format, conversion, truncation etc..

i looked at lib/eal previously and there are... hundreds? of instances so
it's a non-trivial task. the problem i see is somehow getting to a
warnings clean state where we can enable -Werror in the CI pipeline but
at the same time figuring out how to prevent new instances from
appearing until we do.

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Thomas Monjalon]

06/05/2021 00:45, Tyler Retzlaff:
> as a side question, what is the projects stance on getting more warnings
> clean? there are a few not enabled that i'd really like to see e.g.
> format, conversion, truncation etc..
> 
> i looked at lib/eal previously and there are... hundreds? of instances so
> it's a non-trivial task. the problem i see is somehow getting to a
> warnings clean state where we can enable -Werror in the CI pipeline but
> at the same time figuring out how to prevent new instances from
> appearing until we do.

I don't understand the question.
We are already supposed to be warning-free,
and -werror should be enabled in all CI labs.
What are the gaps?

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] examples/rxtx_callbacks: fix port ID format specifier

[Stephen Hemminger]

On Thu,  6 May 2021 00:51:33 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> This fixes -Wformat warning with clang 10.0.0 on Windows.
> 
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> v2: use PRIx16 instead of %u and a cast (Tyler, Thomas).
> 
>  examples/rxtx_callbacks/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
> index b57b2fc6bc..ecc2da2d8c 100644
> --- a/examples/rxtx_callbacks/main.c
> +++ b/examples/rxtx_callbacks/main.c
> @@ -329,7 +329,7 @@ main(int argc, char *argv[])
>  	/* initialize all ports */
>  	RTE_ETH_FOREACH_DEV(portid)
>  		if (port_init(portid, mbuf_pool) != 0)
> -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> +			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIx16"\n",
>  					portid);
>  
>  	if (rte_lcore_count() > 1)

NAK

Just use %u  it works for uint16. Don't start using hex, no other places in
DPDK print port number in hex. 

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] examples/rxtx_callbacks: fix port ID format specifier

[Stephen Hemminger]

On Thu,  6 May 2021 00:54:35 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> This fixes -Wformat warning with clang 10.0.0 on Windows.
> 
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> v3: PRIx16 -> PRIu16
> v2: u% and a cast -> PRIx16 (Tyler, Thomas)
>  examples/rxtx_callbacks/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
> index b57b2fc6bc..192521c3c6 100644
> --- a/examples/rxtx_callbacks/main.c
> +++ b/examples/rxtx_callbacks/main.c
> @@ -329,7 +329,7 @@ main(int argc, char *argv[])
>  	/* initialize all ports */
>  	RTE_ETH_FOREACH_DEV(portid)
>  		if (port_init(portid, mbuf_pool) != 0)
> -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> +			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu16"\n",
>  					portid)

Ok, that matches what is used in rte_ethdev.h

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Tyler Retzlaff]

On Thu, May 06, 2021 at 01:13:07AM +0200, Thomas Monjalon wrote:
> 06/05/2021 00:45, Tyler Retzlaff:
> > as a side question, what is the projects stance on getting more warnings
> > clean? there are a few not enabled that i'd really like to see e.g.
> > format, conversion, truncation etc..
> > 
> > i looked at lib/eal previously and there are... hundreds? of instances so
> > it's a non-trivial task. the problem i see is somehow getting to a
> > warnings clean state where we can enable -Werror in the CI pipeline but
> > at the same time figuring out how to prevent new instances from
> > appearing until we do.
> 
> I don't understand the question.
> We are already supposed to be warning-free,
> and -werror should be enabled in all CI labs.
> What are the gaps?

with the warnings we have enabled -Werror passes, but we don't have all
the warnings we could have that are of value.  it would be great to see
-Wconversion -Wsign-compare -Wsign-conversion passing clean.

i admit it is a lot of work though.

Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix port ID format specifier

[Thomas Monjalon]

12/05/2021 00:34, Tyler Retzlaff:
> On Thu, May 06, 2021 at 01:13:07AM +0200, Thomas Monjalon wrote:
> > 06/05/2021 00:45, Tyler Retzlaff:
> > > as a side question, what is the projects stance on getting more warnings
> > > clean? there are a few not enabled that i'd really like to see e.g.
> > > format, conversion, truncation etc..
> > > 
> > > i looked at lib/eal previously and there are... hundreds? of instances so
> > > it's a non-trivial task. the problem i see is somehow getting to a
> > > warnings clean state where we can enable -Werror in the CI pipeline but
> > > at the same time figuring out how to prevent new instances from
> > > appearing until we do.
> > 
> > I don't understand the question.
> > We are already supposed to be warning-free,
> > and -werror should be enabled in all CI labs.
> > What are the gaps?
> 
> with the warnings we have enabled -Werror passes, but we don't have all
> the warnings we could have that are of value.  it would be great to see
> -Wconversion -Wsign-compare -Wsign-conversion passing clean.
> 
> i admit it is a lot of work though.

-Wsign-compare is enabled for sure, look at config/meson.build.
For the conversion warnings, I don't know.
Do not hesitate to propose new warnings enablement
with a list of warnings it is generating in its current state,
so we can decide whether to enable (and fix) or not.

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] examples/rxtx_callbacks: fix port ID format specifier

[Thomas Monjalon]

06/05/2021 04:56, Stephen Hemminger:
> On Thu,  6 May 2021 00:54:35 +0300
> Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
> 
> > This fixes -Wformat warning with clang 10.0.0 on Windows.
> > 
> > Fixes: f8244c6399d9 ("ethdev: increase port id range")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> > v3: PRIx16 -> PRIu16
> > v2: u% and a cast -> PRIx16 (Tyler, Thomas)
> >  examples/rxtx_callbacks/main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
> > index b57b2fc6bc..192521c3c6 100644
> > --- a/examples/rxtx_callbacks/main.c
> > +++ b/examples/rxtx_callbacks/main.c
> > @@ -329,7 +329,7 @@ main(int argc, char *argv[])
> >  	/* initialize all ports */
> >  	RTE_ETH_FOREACH_DEV(portid)
> >  		if (port_init(portid, mbuf_pool) != 0)
> > -			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu8"\n",
> > +			rte_exit(EXIT_FAILURE, "Cannot init port %"PRIu16"\n",
> >  					portid)
> 
> Ok, that matches what is used in rte_ethdev.h
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Applied, thanks