[11/12] app/flow-perf: fix build with GCC 12
Checks
Commit Message
GCC 12 raises the following warning:
../app/test-flow-perf/main.c: In function ‘start_forwarding’:
../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
terminating nul past the end of the destination
[-Werror=format-overflow=]
1737 | sprintf(p[i++], "%d", (int)n);
| ^
In function ‘pretty_number’,
inlined from ‘packet_per_second_stats’ at
../app/test-flow-perf/main.c:1792:4,
inlined from ‘start_forwarding’ at
../app/test-flow-perf/main.c:1831:3:
[...]
We can simplify this code and rely on libc integer formatting via
this system locales.
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
app/test-flow-perf/main.c | 48 ++++++++-------------------------------
1 file changed, 9 insertions(+), 39 deletions(-)
Comments
On Wed, May 18, 2022 at 12:16:56PM +0200, David Marchand wrote:
> GCC 12 raises the following warning:
>
> ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> terminating nul past the end of the destination
> [-Werror=format-overflow=]
> 1737 | sprintf(p[i++], "%d", (int)n);
> | ^
> In function ‘pretty_number’,
> inlined from ‘packet_per_second_stats’ at
> ../app/test-flow-perf/main.c:1792:4,
> inlined from ‘start_forwarding’ at
> ../app/test-flow-perf/main.c:1831:3:
> [...]
>
> We can simplify this code and rely on libc integer formatting via
> this system locales.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
Good idea.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Hi David,
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, May 18, 2022 1:17 PM
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> ferruh.yigit@xilinx.com; stable@dpdk.org; Wisam Monther
> <wisamm@nvidia.com>
> Subject: [PATCH 11/12] app/flow-perf: fix build with GCC 12
>
> GCC 12 raises the following warning:
>
> ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> terminating nul past the end of the destination
> [-Werror=format-overflow=]
> 1737 | sprintf(p[i++], "%d", (int)n);
> | ^
> In function ‘pretty_number’,
> inlined from ‘packet_per_second_stats’ at
> ../app/test-flow-perf/main.c:1792:4,
> inlined from ‘start_forwarding’ at
> ../app/test-flow-perf/main.c:1831:3:
> [...]
>
> We can simplify this code and rely on libc integer formatting via this system
> locales.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
I've tested the patch and reviewed it, it's working fine, so thank you for that.
One comment
The initial value of 0 is 000
Example:
CMD: ./dpdk-test-flow-perf -n 4 -a <PCI> -- ingress --group=1 --ether --queue --rules-count=200000 --enable-fwd
core tx tx drops rx
------ ---------------- ---------------- ----------------
1 000 000 000
Can you handle this to be single 0 instead of not needed leading zeros?
BRs,
Wisam Jaddo
On Wed, Jun 8, 2022 at 11:03 AM Wisam Monther <wisamm@nvidia.com> wrote:
>
> Hi David,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Wednesday, May 18, 2022 1:17 PM
> > To: dev@dpdk.org
> > Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> > ferruh.yigit@xilinx.com; stable@dpdk.org; Wisam Monther
> > <wisamm@nvidia.com>
> > Subject: [PATCH 11/12] app/flow-perf: fix build with GCC 12
> >
> > GCC 12 raises the following warning:
> >
> > ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> > ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> > terminating nul past the end of the destination
> > [-Werror=format-overflow=]
> > 1737 | sprintf(p[i++], "%d", (int)n);
> > | ^
> > In function ‘pretty_number’,
> > inlined from ‘packet_per_second_stats’ at
> > ../app/test-flow-perf/main.c:1792:4,
> > inlined from ‘start_forwarding’ at
> > ../app/test-flow-perf/main.c:1831:3:
> > [...]
> >
> > We can simplify this code and rely on libc integer formatting via this system
> > locales.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> I've tested the patch and reviewed it, it's working fine, so thank you for that.
> One comment
> The initial value of 0 is 000
>
> Example:
> CMD: ./dpdk-test-flow-perf -n 4 -a <PCI> -- ingress --group=1 --ether --queue --rules-count=200000 --enable-fwd
> core tx tx drops rx
> ------ ---------------- ---------------- ----------------
> 1 000 000 000
>
> Can you handle this to be single 0 instead of not needed leading zeros?
Hum, I don't remember why I added this precision...
This should be just a matter of changing the format from %'16.3s to
%'16s, can you confirm?
On Wed, 18 May 2022 12:16:56 +0200
David Marchand <david.marchand@redhat.com> wrote:
> GCC 12 raises the following warning:
>
> ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> terminating nul past the end of the destination
> [-Werror=format-overflow=]
> 1737 | sprintf(p[i++], "%d", (int)n);
> | ^
> In function ‘pretty_number’,
> inlined from ‘packet_per_second_stats’ at
> ../app/test-flow-perf/main.c:1792:4,
> inlined from ‘start_forwarding’ at
> ../app/test-flow-perf/main.c:1831:3:
> [...]
>
> We can simplify this code and rely on libc integer formatting via
> this system locales.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
Fixes and ends up cleaner.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Hi,
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Wednesday, May 18, 2022 1:17 PM
> > > To: dev@dpdk.org
> > > Cc: NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>;
> > > ferruh.yigit@xilinx.com; stable@dpdk.org; Wisam Monther
> > > <wisamm@nvidia.com>
> > > Subject: [PATCH 11/12] app/flow-perf: fix build with GCC 12
> > >
> > > GCC 12 raises the following warning:
> > >
> > > ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> > > ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> > > terminating nul past the end of the destination
> > > [-Werror=format-overflow=]
> > > 1737 | sprintf(p[i++], "%d", (int)n);
> > > | ^
> > > In function ‘pretty_number’,
> > > inlined from ‘packet_per_second_stats’ at
> > > ../app/test-flow-perf/main.c:1792:4,
> > > inlined from ‘start_forwarding’ at
> > > ../app/test-flow-perf/main.c:1831:3:
> > > [...]
> > >
> > > We can simplify this code and rely on libc integer formatting via
> > > this system locales.
> > >
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> >
> > I've tested the patch and reviewed it, it's working fine, so thank you for
> that.
> > One comment
> > The initial value of 0 is 000
> >
> > Example:
> > CMD: ./dpdk-test-flow-perf -n 4 -a <PCI> -- ingress --group=1 --ether --
> queue --rules-count=200000 --enable-fwd
> > core tx tx drops rx
> > ------ ---------------- ---------------- ----------------
> > 1 000 000 000
> >
> > Can you handle this to be single 0 instead of not needed leading zeros?
>
> Hum, I don't remember why I added this precision...
> This should be just a matter of changing the format from %'16.3s to %'16s,
> can you confirm?
Confirmed, you can go with it. Thanks in advance.
BRs,
Wisam Jaddo
@@ -16,6 +16,7 @@
* gives packet per second measurement.
*/
+#include <locale.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -1713,36 +1714,6 @@ do_tx(struct lcore_info *li, uint16_t cnt, uint16_t tx_port,
rte_pktmbuf_free(li->pkts[i]);
}
-/*
- * Method to convert numbers into pretty numbers that easy
- * to read. The design here is to add comma after each three
- * digits and set all of this inside buffer.
- *
- * For example if n = 1799321, the output will be
- * 1,799,321 after this method which is easier to read.
- */
-static char *
-pretty_number(uint64_t n, char *buf)
-{
- char p[6][4];
- int i = 0;
- int off = 0;
-
- while (n > 1000) {
- sprintf(p[i], "%03d", (int)(n % 1000));
- n /= 1000;
- i += 1;
- }
-
- sprintf(p[i++], "%d", (int)n);
-
- while (i--)
- off += sprintf(buf + off, "%s,", p[i]);
- buf[strlen(buf) - 1] = '\0';
-
- return buf;
-}
-
static void
packet_per_second_stats(void)
{
@@ -1764,7 +1735,6 @@ packet_per_second_stats(void)
uint64_t total_rx_pkts = 0;
uint64_t total_tx_drops = 0;
uint64_t tx_delta, rx_delta, drops_delta;
- char buf[3][32];
int nr_valid_core = 0;
sleep(1);
@@ -1789,10 +1759,8 @@ packet_per_second_stats(void)
tx_delta = li->tx_pkts - oli->tx_pkts;
rx_delta = li->rx_pkts - oli->rx_pkts;
drops_delta = li->tx_drops - oli->tx_drops;
- printf("%6d %16s %16s %16s\n", i,
- pretty_number(tx_delta, buf[0]),
- pretty_number(drops_delta, buf[1]),
- pretty_number(rx_delta, buf[2]));
+ printf("%6d %'16.3"PRId64" %'16.3"PRId64" %'16.3"PRId64"\n",
+ i, tx_delta, drops_delta, rx_delta);
total_tx_pkts += tx_delta;
total_rx_pkts += rx_delta;
@@ -1803,10 +1771,9 @@ packet_per_second_stats(void)
}
if (nr_valid_core > 1) {
- printf("%6s %16s %16s %16s\n", "total",
- pretty_number(total_tx_pkts, buf[0]),
- pretty_number(total_tx_drops, buf[1]),
- pretty_number(total_rx_pkts, buf[2]));
+ printf("%6s %'16.3"PRId64" %'16.3"PRId64" %'16.3"PRId64"\n",
+ "total", total_tx_pkts, total_tx_drops,
+ total_rx_pkts);
nr_lines += 1;
}
@@ -2139,6 +2106,9 @@ main(int argc, char **argv)
if (argc > 1)
args_parse(argc, argv);
+ /* For more fancy, localised integer formatting. */
+ setlocale(LC_NUMERIC, "");
+
init_port();
nb_lcores = rte_lcore_count();