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
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?
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.
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>
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.
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
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
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.
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?
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.
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>
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.
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.
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