examples: format truncation bugs
cover examples: format truncation bugs
Enable format-overflow warnings in examples and fix the bugs.
Stephen Hemminger (3):
examples/server_node_efd: fix format overflow
examples/vdpa: fix format overflow warning
examples: re-enable format truncation warning
examples/meson.build | 5 ++--- examples/server_node_efd/efd_server/main.c | 2 +- examples/vdpa/main.c | 12 ++++++------ 3 files changed, 9 insertions(+), 10 deletions(-)
Signature · 1 line
2.51.0
PATCH 1/3 examples/server_node_efd: fix format overflow
If format-truncation is enabled, the compiler detects a string overflow since the snprintf() is putting ethernet address and new line in buffer only sized for the address.
--- examples/server_node_efd/efd_server/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
examples/server_node_efd/efd_server/main.c +1 -1
@@ -68,7 +68,7 @@ static const char * get_printable_mac_addr(uint16_t port) {static const char err_address[] ="00:00:00:00:00:00" ; -static char addresses[RTE_MAX_ETHPORTS ][sizeof (err_address)]; +static char addresses[RTE_MAX_ETHPORTS ][RTE_ETHER_ADDR_FMT_SIZE +1 ];struct rte_ether_addr mac;int ret;
Signature · 1 line
2.51.0
PATCH 2/3 examples/vdpa: fix format overflow warning
In reply to Stephen HemmingerThe ifname is limited to 128 characters, but it would allow up to 128 characters as prefix then could overflow creating ifname.
Change to limit path prefix to 124 (128 - sizeof("1024")) to avoid possible format overflow
--- examples/vdpa/main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
examples/vdpa/main.c +5 -4
@@ -22,6 +22,8 @@#define MAX_PATH_LEN 128#define MAX_VDPA_SAMPLE_PORTS 1024 +#define stringify (x) (#x) +#define MAX_VDPA_STR_LEN sizeof(stringify(MAX_VDPA_SAMPLE_PORTS))#define RTE_LOGTYPE_VDPA RTE_LOGTYPE_USER1struct vdpa_port {
@@ -36,7 +38,7 @@ struct vdpa_port {static struct vdpa_port vports[MAX_VDPA_SAMPLE_PORTS ]; -static char iface[MAX_PATH_LEN ]; +static char iface[MAX_PATH_LEN -MAX_VDPA_STR_LEN ];static int devcnt;static int interactive;static int client_mode;
@@ -74,9 +76,8 @@ parse_args(int argc, char **argv)break ;/* long options */ case 0 : -if (strncmp (long_option[idx].name,"iface" , -MAX_PATH_LEN ) ==0 ) { -rte_strscpy (iface, optarg,MAX_PATH_LEN ); +if (!strcmp (long_option[idx].name,"iface" )) { +rte_strscpy (iface, optarg,sizeof (iface));printf ("iface %s\n" , iface); }if (!strcmp (long_option[idx].name,"interactive" )) {
Signature · 1 line
2.51.0
PATCH 3/3 examples: re-enable format truncation warning
In reply to Stephen HemmingerThere were several hidden bugs in examples because format truncation warning was disabled.
--- examples/meson.build | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
examples/meson.build +2 -3
@@ -78,10 +78,9 @@ else examples = get_option('examples').split(',') allow_skips = false # error out if we can't build a requested app endif + default_cflags = machine_args -if cc.has_argument('-Wno-format-truncation') - default_cflags += '-Wno-format-truncation' -endif + default_ldflags = dpdk_extra_ldflags if get_option('default_library') == 'static' and not is_windows default_ldflags += ['-Wl,--export-dynamic']
Signature · 1 line
2.51.0
v2 cover fix format-truncation warnings
In reply to Stephen HemmingerEnable and fix format-truncation warnings in examples and tests.
v2 - fix build warnings (more fixes needed)
- do test as well (could be separate series?)
Stephen Hemminger (7):
examples/server_node_efd: fix format overflow examples/vdpa: fix format overflow warning examples/ip_reassembly: add check before formatting name
examples: re-enable format truncation warning
test: refactor file prefix arg handling
test: increase size of memzone name
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/test_eal_flags.c | 161 ++++++++------------- app/test/test_memzone.c | 2 +- examples/ip_reassembly/main.c | 7 + examples/meson.build | 5 +- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 +- examples/vdpa/main.c | 9 +- 8 files changed, 77 insertions(+), 113 deletions(-)
Signature · 1 line
2.51.0
PATCH v2 1/7 examples/server_node_efd: fix format overflow
If format-truncation is enabled, the compiler detects a string overflow since the snprintf() is putting ethernet address and new line in buffer only sized for the address.
Since get_rx_queue_name() is used to create a ring name. The buffer should be sized to be a valid ring name. This fixes some format-overflow warnings and also corrects for future errors.
--- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
examples/server_node_efd/efd_server/main.c +1 -1
@@ -68,7 +68,7 @@ static const char * get_printable_mac_addr(uint16_t port) {static const char err_address[] ="00:00:00:00:00:00" ; -static char addresses[RTE_MAX_ETHPORTS ][sizeof (err_address)]; +static char addresses[RTE_MAX_ETHPORTS ][RTE_ETHER_ADDR_FMT_SIZE +1 ];struct rte_ether_addr mac;int ret;
examples/server_node_efd/shared/common.h +2 -1
@@ -58,8 +58,9 @@ get_rx_queue_name(unsigned int id)/* * Buffer for return value. Size calculated by %u being replaced * by maximum 3 digits (plus an extra byte for safety) +* Used as ring name, so upper limit is ring name size. */ -static char buffer[sizeof (MP_NODE_RXQ_NAME ) +2 ]; +static char buffer[RTE_RING_NAMESIZE ];snprintf (buffer,sizeof (buffer),MP_NODE_RXQ_NAME , id);return buffer;
Signature · 1 line
2.51.0
PATCH v2 2/7 examples/vdpa: fix format overflow warning
In reply to Stephen HemmingerThe ifname is limited to 128 characters, but it would allow up to 128 characters as prefix then could overflow creating ifname.
Change to limit path prefix to 124 (128 - sizeof("1024")) to avoid possible format overflow
--- examples/vdpa/main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
examples/vdpa/main.c +5 -4
@@ -22,6 +22,8 @@#define MAX_PATH_LEN 128#define MAX_VDPA_SAMPLE_PORTS 1024 +#define stringify (x) (#x) +#define MAX_VDPA_STR_LEN sizeof(stringify(MAX_VDPA_SAMPLE_PORTS))#define RTE_LOGTYPE_VDPA RTE_LOGTYPE_USER1struct vdpa_port {
@@ -36,7 +38,7 @@ struct vdpa_port {static struct vdpa_port vports[MAX_VDPA_SAMPLE_PORTS ]; -static char iface[MAX_PATH_LEN ]; +static char iface[MAX_PATH_LEN -MAX_VDPA_STR_LEN ];static int devcnt;static int interactive;static int client_mode;
@@ -74,9 +76,8 @@ parse_args(int argc, char **argv)break ;/* long options */ case 0 : -if (strncmp (long_option[idx].name,"iface" , -MAX_PATH_LEN ) ==0 ) { -rte_strscpy (iface, optarg,MAX_PATH_LEN ); +if (!strcmp (long_option[idx].name,"iface" )) { +rte_strscpy (iface, optarg,sizeof (iface));printf ("iface %s\n" , iface); }if (!strcmp (long_option[idx].name,"interactive" )) {
Signature · 1 line
2.51.0
Re: [PATCH v2 2/7] examples/vdpa: fix format overflow warning
Quoted 4 lines · Stephen Hemminger
The ifname is limited to 128 characters, but it would allow up to 128 characters as prefix then could overflow creating ifname.
Change to limit path prefix to 124 (128 -
sizeof("1024"))
nit: sizeof("1024") == 5, because of the \0.
Quoted 7 lines
to avoid possible format overflow
Fixes 38f8ab0bbc8d ("vhost: make vDPA framework bus agnostic")Signed-off-by Stephen Hemminger <[email protected]>
I assume this was found by enabling compiler warnings. It would be good to include the actual warning message in the commit log, given that the patch doesn't actually modify the line at which the overflow occurs.
Quoted 9 lines
--- examples/vdpa/main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)examples/vdpa/main.c +0
-0
examples/vdpa/main.c line 26
Quoted 5 lines
examples/vdpa/main.c +2
-0@@ -22,6 +22,8 @@#define MAX_PATH_LEN 128#define MAX_VDPA_SAMPLE_PORTS 1024 +#define stringify (x) (#x) +#define MAX_VDPA_STR_LEN sizeof(stringify(MAX_VDPA_SAMPLE_PORTS))
We have RTE_STR() and _RTE_STR() in rte_common.h
Quoted 13 lines
examples/vdpa/main.c +1
-1#define RTE_LOGTYPE_VDPA RTE_LOGTYPE_USER1struct vdpa_port {@@ -36,7 +38,7 @@ struct vdpa_port {static struct vdpa_port vports[MAX_VDPA_SAMPLE_PORTS ]; -static char iface[MAX_PATH_LEN ]; +static char iface[MAX_PATH_LEN -MAX_VDPA_STR_LEN ];static int devcnt;static int interactive;static int client_mode;
examples/vdpa/main.c parse_args(int argc, char **argv) · line 80
Quoted 8 lines
examples/vdpa/main.c +2
-3@@ -74,9 +76,8 @@ parse_args(int argc, char **argv)break ;/* long options */ case 0 : -if (strncmp (long_option[idx].name,"iface" , -MAX_PATH_LEN ) ==0 ) { -rte_strscpy (iface, optarg,MAX_PATH_LEN ); +if (!strcmp (long_option[idx].name,"iface" )) { +rte_strscpy (iface, optarg,sizeof (iface));
I realise that the rte_strscpy function is what was here before, but given that we are not checking the return value, there is no reason to choose the dpdk-only strscpy function over the more-standard strlcpy.
Quoted 6 lines
examples/vdpa/main.c +0
-0printf ("iface %s\n" , iface); }if (!strcmp (long_option[idx].name,"interactive" )) {Signature · 1 line
2.51.0
PATCH v2 3/7 examples/ip_reassembly: add check before formatting name
In reply to Stephen HemmingerIn theory, lcore and queue could be so large that mbuf pool name could overflow. But that can never happen since lcore and queue will be in range. Add a check so that static tools know that.
--- examples/ip_reassembly/main.c | 7 +++++++ 1 file changed, 7 insertions(+)
examples/ip_reassembly/main.c +7 -0
@@ -884,6 +884,13 @@ setup_queue_tbl(struct rx_queue *rxq, uint32_t lcore, uint32_t queue) nb_mbuf =RTE_MAX (nb_mbuf, (uint32_t )NB_MBUF ); +/* Should never happen but check so that pool name won't be too long. */ +if (lcore >RTE_MAX_LCORE || queue >RTE_MAX_QUEUES_PER_PORT ) { +RTE_LOG (ERR ,IP_RSMBL ,"invalid lcore %u or queue %u" , + lcore, queue); +return -1 ; + } +snprintf (buf,sizeof (buf),"mbuf_pool_%u_%u" , lcore, queue); rxq->pool = rte_pktmbuf_pool_create(buf, nb_mbuf, MEMPOOL_CACHE_SIZE, 0,
Signature · 1 line
2.51.0
RE: [PATCH v2 3/7] examples/ip_reassembly: add check before formatting name
Quoted 30 lines
In theory, lcore and queue could be so large that mbuf pool name could overflow. But that can never happen since lcore and queue will be in range. Add a check so that static tools know that.
Signed-off-by Stephen Hemminger <[email protected]>--- examples/ip_reassembly/main.c | 7 +++++++ 1 file changed, 7 insertions(+)examples/ip_reassembly/main.c +7
-0@@ -884,6 +884,13 @@ setup_queue_tbl(struct rx_queue *rxq, uint32_t lcore, uint32_t queue) nb_mbuf =RTE_MAX (nb_mbuf, (uint32_t )NB_MBUF ); +/* Should never happen but check so that pool name won't be too long. */ +if (lcore >RTE_MAX_LCORE || queue >RTE_MAX_QUEUES_PER_PORT ) { +RTE_LOG (ERR ,IP_RSMBL ,"invalid lcore %u or queue %u" , + lcore, queue); +return -1 ; + } +snprintf (buf,sizeof (buf),"mbuf_pool_%u_%u" , lcore, queue);
Quoted 3 lines
rxq->pool= rte_pktmbuf_pool_create(buf,nb_mbuf,MEMPOOL_CACHE_SIZE, 0,
--
Quoted 1 line
2.51.0
Re: [PATCH v2 3/7] examples/ip_reassembly: add check before formatting name
In reply to Stephen HemmingerQuoted 6 lines · Stephen Hemminger
In theory, lcore and queue could be so large that mbuf pool name could overflow. But that can never happen since lcore and queue will be in range. Add a check so that static tools know that.
Signed-off-by Stephen Hemminger <[email protected]>---
Again, maybe include the warning omitted by the compiler
Quoted 24 lines
examples/ip_reassembly/main.c | 7 +++++++ 1 file changed, 7 insertions(+)examples/ip_reassembly/main.c +7
-0@@ -884,6 +884,13 @@ setup_queue_tbl(struct rx_queue *rxq, uint32_t lcore, uint32_t queue) nb_mbuf =RTE_MAX (nb_mbuf, (uint32_t )NB_MBUF ); +/* Should never happen but check so that pool name won't be too long. */ +if (lcore >RTE_MAX_LCORE || queue >RTE_MAX_QUEUES_PER_PORT ) { +RTE_LOG (ERR ,IP_RSMBL ,"invalid lcore %u or queue %u" , + lcore, queue); +return -1 ; + } +snprintf (buf,sizeof (buf),"mbuf_pool_%u_%u" , lcore, queue); rxq->pool = rte_pktmbuf_pool_create(buf, nb_mbuf, MEMPOOL_CACHE_SIZE, 0,Signature · 1 line
2.51.0
PATCH v2 4/7 examples: re-enable format truncation warning
In reply to Stephen HemmingerThere were several hidden bugs in examples because format truncation warning was disabled.
--- examples/meson.build | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
examples/meson.build +2 -3
@@ -78,10 +78,9 @@ else examples = get_option('examples').split(',') allow_skips = false # error out if we can't build a requested app endif + default_cflags = machine_args -if cc.has_argument('-Wno-format-truncation') - default_cflags += '-Wno-format-truncation' -endif + default_ldflags = dpdk_extra_ldflags if get_option('default_library') == 'static' and not is_windows default_ldflags += ['-Wl,--export-dynamic']
Signature · 1 line
2.51.0
Re: [PATCH v2 4/7] examples: re-enable format truncation warning
Quoted 5 lines · Stephen Hemminger
There were several hidden bugs in examples because format truncation warning was disabled.
Signed-off-by Stephen Hemminger <[email protected]>---
Quoted 23 lines
examples/meson.build | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)examples/meson.build +2
-3@@ -78,10 +78,9 @@ else examples = get_option('examples').split(',') allow_skips = false # error out if we can't build a requested app endif + default_cflags = machine_args -if cc.has_argument('-Wno-format-truncation') - default_cflags += '-Wno-format-truncation' -endif + default_ldflags = dpdk_extra_ldflags if get_option('default_library') == 'static' and not is_windows default_ldflags += ['-Wl,--export-dynamic']Signature · 1 line
2.51.0
PATCH v2 5/7 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/test_eal_flags.c | 161 ++++++++++++++------------------------ 1 file changed, 59 insertions(+), 102 deletions(-)
app/test/test_eal_flags.c +59 -102
@@ -121,6 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" +#define file_prefix "--file-prefix" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -312,26 +314,40 @@ get_number_of_sockets(void) }#endif -/* -* Test that the app doesn't run with invalid allow option. -* Final tests ensures it does run with valid options as sanity check (one -* test for with Domain+BDF, second for just with BDF) -*/ +/* Get the file_prefix xommand line argument */ static int -test_allow_flag (void ) +get_file_prefix (char *prefix,size_t size) { -unsigned i;#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; +RTE_SET_USED (len); + *prefix =0 ; +return 0 ;#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; +char tmp[PATH_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );return -1 ; } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); +snprintf (prefix, size,"%s=%s" , file_prefix, tmp); +return 0 ;#endif +} + +/* +* Test that the app doesn't run with invalid allow option. +* Final tests ensures it does run with valid options as sanity check (one +* test for with Domain+BDF, second for just with BDF) +*/ +static int +test_allow_flag (void ) +{ +unsigned i; +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;const char *wlinval[][7 ] = { {prgname, prefix, mp_flag,
@@ -387,17 +403,10 @@ test_allow_flag(void)static int test_invalid_b_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;const char *blinval[][5 ] = { {prgname, prefix, mp_flag,"-b" ,"error" },
@@ -491,17 +500,10 @@ test_invalid_vdev_flag(void)static int test_invalid_r_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;const char *rinval[][5 ] = { {prgname, prefix, mp_flag,"-r" ,"error" },
@@ -535,17 +537,10 @@ test_invalid_r_flag(void)static int test_missing_c_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;/* -c flag but no coremask value */ const char *argv1[] = { prgname, prefix, mp_flag,"-c" };
@@ -694,17 +689,10 @@ test_missing_c_flag(void)static int test_main_lcore_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char *prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;if (!rte_lcore_is_enabled (0 ) || !rte_lcore_is_enabled (1 ))return TEST_SKIPPED ;
@@ -751,17 +739,10 @@ test_main_lcore_flag(void)static int test_invalid_n_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;/* -n flag but no value */ const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
@@ -803,18 +784,10 @@ test_invalid_n_flag(void)static int test_no_hpet_flag (void ) { -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX +sizeof (file_prefix)]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else -char tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;/* With --no-hpet */ const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
@@ -913,17 +886,14 @@ test_misc_flags(void)const char * prefix ="" ;const char * nosh_prefix ="" ;#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; +char prefix[PATH_MAX +sizeof (file_prefix)];const char * nosh_prefix ="--file-prefix=noshconf" ;FILE * hugedir_handle =NULL ;char line[PATH_MAX ] = {0 };unsigned i, isempty =1 ; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;/* * get first valid hugepage path
@@ -1239,16 +1209,10 @@ test_file_prefix(void)* run in legacy mode, and not present at all after run in default * mem mode */ -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX +sizeof (file_prefix)]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else -if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -#endif +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;/* this should fail unless the test itself is run with "memtest" prefix */ const char *argv0[] = {prgname, mp_flag,"-m" ,
@@ -1525,17 +1489,10 @@ populate_socket_mem_param(int num_sockets, const char *mem,static int test_memory_flags (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); -return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif +char prefix[PATH_MAX +sizeof (file_prefix)]; + +if (get_file_prefix (prefix,sizeof (prefix)) !=0 ) +return TEST_FAILED ;/* valid -m flag and mp flag */ const char *argv0[] = {prgname, prefix, mp_flag,
Signature · 1 line
2.51.0
PATCH v2 6/7 test: increase size of memzone name
In reply to Stephen HemmingerThe tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
Re: [PATCH v2 6/7] test: increase size of memzone name
Quoted 5 lines · Stephen Hemminger
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as
RTE_MEMZONE_NAMESIZEso use that.Signed-off-by Stephen Hemminger <[email protected]>
Quoted 20 lines
--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)app/test/test_memzone.c +1
-1@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,Signature · 1 line
2.51.0
PATCH v2 7/7 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
Re: [PATCH v2 7/7] test: re-enable format-truncation warnings
Quoted 4 lines · Stephen Hemminger
Tests should be built with warnings.
Signed-off-by Stephen Hemminger <[email protected]>---
Quoted 18 lines
app/test/meson.build | 1 - 1 file changed, 1 deletion(-)app/test/meson.build +0
-1@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flagsSignature · 1 line
2.51.0
v3 cover examples: fix format-truncation warnings
In reply to Stephen HemmingerEnable and fix format-truncation warnings in examples
v3 - examples only, update commit messages
Stephen Hemminger (4):
examples/server_node_efd: fix format overflow examples/vdpa: fix format overflow warning examples/ip_reassembly: add check before formatting name
examples: enable format truncation warning
examples/ip_reassembly/main.c | 7 +++++++ examples/meson.build | 5 ++--- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 ++- examples/vdpa/main.c | 9 +++++---- 5 files changed, 17 insertions(+), 9 deletions(-)
Signature · 1 line
2.51.0
PATCH v3 1/4 examples/server_node_efd: fix format overflow
If format-truncation is enabled, the compiler detects a string overflow since the snprintf() is putting ethernet address and new line in buffer only sized for the address.
Since get_rx_queue_name() is used to create a ring name. The buffer should be sized to be a valid ring name. This fixes some format-overflow warnings and also corrects for future errors.
In file included from ../examples/server_node_efd/efd_server/main.c:31:
In function ‘get_printable_mac_addr’,
inlined from ‘do_stats_display’ at ../examples/server_node_efd/efd_server/main.c:136:3:../lib/net/rte_ether.h:248:36: warning: ‘snprintf’ output truncated before the last format character [-Wformat-truncation=]
248 | #define RTE_ETHER_ADDR_PRT_FMT "%02X:%02X:%02X:%02X:%02X:%02X"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../examples/server_node_efd/efd_server/main.c:86:33: note: in expansion of macro ‘RTE_ETHER_ADDR_PRT_FMT’
86 | RTE_ETHER_ADDR_PRT_FMT "\n",
| ^~~~~~~~~~~~~~~~~~~~~~
../examples/server_node_efd/efd_server/main.c: In function ‘do_stats_display’:
../examples/server_node_efd/efd_server/main.c:86:59: note: format string is defined here
86 | RTE_ETHER_ADDR_PRT_FMT "\n",
|--- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
examples/server_node_efd/efd_server/main.c +1 -1
@@ -68,7 +68,7 @@ static const char * get_printable_mac_addr(uint16_t port) {static const char err_address[] ="00:00:00:00:00:00" ; -static char addresses[RTE_MAX_ETHPORTS ][sizeof (err_address)]; +static char addresses[RTE_MAX_ETHPORTS ][RTE_ETHER_ADDR_FMT_SIZE +1 ];struct rte_ether_addr mac;int ret;
examples/server_node_efd/shared/common.h +2 -1
@@ -58,8 +58,9 @@ get_rx_queue_name(unsigned int id)/* * Buffer for return value. Size calculated by %u being replaced * by maximum 3 digits (plus an extra byte for safety) +* Used as ring name, so upper limit is ring name size. */ -static char buffer[sizeof (MP_NODE_RXQ_NAME ) +2 ]; +static char buffer[RTE_RING_NAMESIZE ];snprintf (buffer,sizeof (buffer),MP_NODE_RXQ_NAME , id);return buffer;
Signature · 1 line
2.51.0
Re: [PATCH v3 1/4] examples/server_node_efd: fix format overflow
Quoted 23 lines · Stephen Hemminger
If format-truncation is enabled, the compiler detects a string overflow since the
snprintf()is putting ethernet address and new line in buffer only sized for the address.Since
get_rx_queue_name()is used to create a ring name. The buffer should be sized to be a valid ring name. This fixes some format-overflow warnings and also corrects for future errors.In file included from
../examples/server_node_efd/efd_server/main.c:31:
In function ‘get_printable_mac_addr’,
inlined from ‘do_stats_display’ at../examples/server_node_efd/efd_server/main.c:136:3:../lib/net/rte_ether.h:248:36: warning: ‘snprintf’ output truncated before the last format character [-Wformat-truncation=]248 | #define RTE_ETHER_ADDR_PRT_FMT "%02X:%02X:%02X:%02X:%02X:%02X" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../examples/server_node_efd/efd_server/main.c:86:33: note: in expansion of macro ‘RTE_ETHER_ADDR_PRT_FMT’ 86 | RTE_ETHER_ADDR_PRT_FMT "\n", | ^~~~~~~~~~~~~~~~~~~~~~ ../examples/server_node_efd/efd_server/main.c: In function ‘do_stats_display’: ../examples/server_node_efd/efd_server/main.c:86:59: note: format string is defined here 86 | RTE_ETHER_ADDR_PRT_FMT "\n", |
Quoted 4 lines
Fixes 39aad0e88c58 ("examples/flow_distributor: new example to demonstrate EFD")Signed-off-by Stephen Hemminger <[email protected]>
Quoted 36 lines
--- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)examples/server_node_efd/efd_server/main.c +1
-1@@ -68,7 +68,7 @@ static const char * get_printable_mac_addr(uint16_t port) {static const char err_address[] ="00:00:00:00:00:00" ; -static char addresses[RTE_MAX_ETHPORTS ][sizeof (err_address)]; +static char addresses[RTE_MAX_ETHPORTS ][RTE_ETHER_ADDR_FMT_SIZE +1 ];struct rte_ether_addr mac;int ret;examples/server_node_efd/shared/common.h +2
-1@@ -58,8 +58,9 @@ get_rx_queue_name(unsigned int id)/* * Buffer for return value. Size calculated by %u being replaced * by maximum 3 digits (plus an extra byte for safety) +* Used as ring name, so upper limit is ring name size. */ -static char buffer[sizeof (MP_NODE_RXQ_NAME ) +2 ]; +static char buffer[RTE_RING_NAMESIZE ];snprintf (buffer,sizeof (buffer),MP_NODE_RXQ_NAME , id);return buffer;Signature · 1 line
2.51.0
PATCH v3 2/4 examples/vdpa: fix format overflow warning
In reply to Stephen HemmingerThe ifname is limited to 128 characters, but it would allow up to 128 characters as prefix then could overflow creating ifname.
Change to limit path prefix (iface) to 123 (128 - sizeof("1024")) to avoid possible format overflow
../examples/vdpa/main.c:501:76: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
501 | snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
| ^../examples/vdpa/main.c:501:25: note: ‘snprintf’ output between 2 and 139 bytes into a destination of size 128
501 | snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
502 | iface, devcnt);
|--- examples/vdpa/main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
examples/vdpa/main.c +5 -4
@@ -22,6 +22,8 @@#define MAX_PATH_LEN 128#define MAX_VDPA_SAMPLE_PORTS 1024 +#define stringify (x) (#x) +#define MAX_VDPA_STR_LEN sizeof(stringify(MAX_VDPA_SAMPLE_PORTS))#define RTE_LOGTYPE_VDPA RTE_LOGTYPE_USER1struct vdpa_port {
@@ -36,7 +38,7 @@ struct vdpa_port {static struct vdpa_port vports[MAX_VDPA_SAMPLE_PORTS ]; -static char iface[MAX_PATH_LEN ]; +static char iface[MAX_PATH_LEN -MAX_VDPA_STR_LEN ];static int devcnt;static int interactive;static int client_mode;
@@ -74,9 +76,8 @@ parse_args(int argc, char **argv)break ;/* long options */ case 0 : -if (strncmp (long_option[idx].name,"iface" , -MAX_PATH_LEN ) ==0 ) { -rte_strscpy (iface, optarg,MAX_PATH_LEN ); +if (!strcmp (long_option[idx].name,"iface" )) { +rte_strscpy (iface, optarg,sizeof (iface));printf ("iface %s\n" , iface); }if (!strcmp (long_option[idx].name,"interactive" )) {
Signature · 1 line
2.51.0
Re: [PATCH v3 2/4] examples/vdpa: fix format overflow warning
15/11/2025 20:36, Stephen Hemminger:
examples/vdpa/main.c line 26
Quoted 2 lines
examples/vdpa/main.c +2
-0+#define stringify (x) (#x) +#define MAX_VDPA_STR_LEN sizeof(stringify(MAX_VDPA_SAMPLE_PORTS))
Can you use RTE_STR?
Re: [PATCH v3 2/4] examples/vdpa: fix format overflow warning
Quoted 6 lines · Thomas Monjalon
15/11/2025 20:36, Stephen Hemminger:
examples/vdpa/main.c line 26
Quoted 2 lines
examples/vdpa/main.c +2
-0+#define stringify (x) (#x) +#define MAX_VDPA_STR_LEN sizeof(stringify(MAX_VDPA_SAMPLE_PORTS))Can you use
RTE_STR?
+1 I had same comment on v2.
PATCH v3 3/4 examples/ip_reassembly: add check before formatting name
In reply to Stephen HemmingerIn theory, lcore and queue could be so large that mbuf pool name could overflow. But that can never happen since lcore and queue will be in range. Add a check so that static tools know that. Format overflow warnings seen on older versions of Gcc only.
--- examples/ip_reassembly/main.c | 7 +++++++ 1 file changed, 7 insertions(+)
examples/ip_reassembly/main.c +7 -0
@@ -884,6 +884,13 @@ setup_queue_tbl(struct rx_queue *rxq, uint32_t lcore, uint32_t queue) nb_mbuf =RTE_MAX (nb_mbuf, (uint32_t )NB_MBUF ); +/* Should never happen but check so that pool name won't be too long. */ +if (lcore >RTE_MAX_LCORE || queue >RTE_MAX_QUEUES_PER_PORT ) { +RTE_LOG (ERR ,IP_RSMBL ,"invalid lcore %u or queue %u" , + lcore, queue); +return -1 ; + } +snprintf (buf,sizeof (buf),"mbuf_pool_%u_%u" , lcore, queue); rxq->pool = rte_pktmbuf_pool_create(buf, nb_mbuf, MEMPOOL_CACHE_SIZE, 0,
Signature · 1 line
2.51.0
PATCH v3 4/4 examples: enable format truncation warning
In reply to Stephen HemmingerThere were several hidden bugs in examples because format truncation warning was disabled.
--- examples/meson.build | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
examples/meson.build +2 -3
@@ -78,10 +78,9 @@ else examples = get_option('examples').split(',') allow_skips = false # error out if we can't build a requested app endif + default_cflags = machine_args -if cc.has_argument('-Wno-format-truncation') - default_cflags += '-Wno-format-truncation' -endif + default_ldflags = dpdk_extra_ldflags if get_option('default_library') == 'static' and not is_windows default_ldflags += ['-Wl,--export-dynamic']
Signature · 1 line
2.51.0
v4 cover examples: fix format-truncation errors
In reply to Stephen HemmingerEnabling format-truncation warning in examples show several possible overflows that are easily addressed.
v4 - use RTE_STR()
Stephen Hemminger (4):
examples/server_node_efd: fix format overflow examples/vdpa: fix format overflow warning examples/ip_reassembly: add check before formatting name
examples: enable format truncation warning
examples/ip_reassembly/main.c | 7 +++++++ examples/meson.build | 5 ++--- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 ++- examples/vdpa/main.c | 8 ++++---- 5 files changed, 16 insertions(+), 9 deletions(-)
Signature · 1 line
2.51.0
PATCH v4 1/4 examples/server_node_efd: fix format overflow
If format-truncation is enabled, the compiler detects a string overflow since the snprintf() is putting ethernet address and new line in buffer only sized for the address.
Since get_rx_queue_name() is used to create a ring name. The buffer should be sized to be a valid ring name. This fixes some format-overflow warnings and also corrects for future errors.
In file included from ../examples/server_node_efd/efd_server/main.c:31:
In function ‘get_printable_mac_addr’,
inlined from ‘do_stats_display’ at ../examples/server_node_efd/efd_server/main.c:136:3:../lib/net/rte_ether.h:248:36: warning: ‘snprintf’ output truncated before the last format character [-Wformat-truncation=]
248 | #define RTE_ETHER_ADDR_PRT_FMT "%02X:%02X:%02X:%02X:%02X:%02X"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../examples/server_node_efd/efd_server/main.c:86:33: note: in expansion of macro ‘RTE_ETHER_ADDR_PRT_FMT’
86 | RTE_ETHER_ADDR_PRT_FMT "\n",
| ^~~~~~~~~~~~~~~~~~~~~~
../examples/server_node_efd/efd_server/main.c: In function ‘do_stats_display’:
../examples/server_node_efd/efd_server/main.c:86:59: note: format string is defined here
86 | RTE_ETHER_ADDR_PRT_FMT "\n",
|--- examples/server_node_efd/efd_server/main.c | 2 +- examples/server_node_efd/shared/common.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
examples/server_node_efd/efd_server/main.c +1 -1
@@ -68,7 +68,7 @@ static const char * get_printable_mac_addr(uint16_t port) {static const char err_address[] ="00:00:00:00:00:00" ; -static char addresses[RTE_MAX_ETHPORTS ][sizeof (err_address)]; +static char addresses[RTE_MAX_ETHPORTS ][RTE_ETHER_ADDR_FMT_SIZE +1 ];struct rte_ether_addr mac;int ret;
examples/server_node_efd/shared/common.h +2 -1
@@ -58,8 +58,9 @@ get_rx_queue_name(unsigned int id)/* * Buffer for return value. Size calculated by %u being replaced * by maximum 3 digits (plus an extra byte for safety) +* Used as ring name, so upper limit is ring name size. */ -static char buffer[sizeof (MP_NODE_RXQ_NAME ) +2 ]; +static char buffer[RTE_RING_NAMESIZE ];snprintf (buffer,sizeof (buffer),MP_NODE_RXQ_NAME , id);return buffer;
Signature · 1 line
2.51.0
PATCH v4 2/4 examples/vdpa: fix format overflow warning
In reply to Stephen HemmingerThe ifname is limited to 128 characters, but it would allow up to 128 characters as prefix then could overflow creating ifname.
Change to limit path prefix (iface) to 123 (128 - sizeof("1024")) to avoid possible format overflow
../examples/vdpa/main.c:501:76: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
501 | snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
| ^../examples/vdpa/main.c:501:25: note: ‘snprintf’ output between 2 and 139 bytes into a destination of size 128
501 | snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
502 | iface, devcnt);
|--- examples/vdpa/main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
examples/vdpa/main.c +4 -4
@@ -22,6 +22,7 @@#define MAX_PATH_LEN 128#define MAX_VDPA_SAMPLE_PORTS 1024 +#define MAX_VDPA_STR_LEN sizeof(RTE_STR(MAX_VDPA_SAMPLE_PORTS))#define RTE_LOGTYPE_VDPA RTE_LOGTYPE_USER1struct vdpa_port {
@@ -36,7 +37,7 @@ struct vdpa_port {static struct vdpa_port vports[MAX_VDPA_SAMPLE_PORTS ]; -static char iface[MAX_PATH_LEN ]; +static char iface[MAX_PATH_LEN -MAX_VDPA_STR_LEN ];static int devcnt;static int interactive;static int client_mode;
@@ -74,9 +75,8 @@ parse_args(int argc, char **argv)break ;/* long options */ case 0 : -if (strncmp (long_option[idx].name,"iface" , -MAX_PATH_LEN ) ==0 ) { -rte_strscpy (iface, optarg,MAX_PATH_LEN ); +if (!strcmp (long_option[idx].name,"iface" )) { +rte_strscpy (iface, optarg,sizeof (iface));printf ("iface %s\n" , iface); }if (!strcmp (long_option[idx].name,"interactive" )) {
Signature · 1 line
2.51.0
PATCH v4 3/4 examples/ip_reassembly: add check before formatting name
In reply to Stephen HemmingerIn theory, lcore and queue could be so large that mbuf pool name could overflow. But that can never happen since lcore and queue will be in range. Add a check so that static tools know that. Format overflow warnings seen on older versions of Gcc only.
--- examples/ip_reassembly/main.c | 7 +++++++ 1 file changed, 7 insertions(+)
examples/ip_reassembly/main.c +7 -0
@@ -884,6 +884,13 @@ setup_queue_tbl(struct rx_queue *rxq, uint32_t lcore, uint32_t queue) nb_mbuf =RTE_MAX (nb_mbuf, (uint32_t )NB_MBUF ); +/* Should never happen but check so that pool name won't be too long. */ +if (lcore >RTE_MAX_LCORE || queue >RTE_MAX_QUEUES_PER_PORT ) { +RTE_LOG (ERR ,IP_RSMBL ,"invalid lcore %u or queue %u" , + lcore, queue); +return -1 ; + } +snprintf (buf,sizeof (buf),"mbuf_pool_%u_%u" , lcore, queue); rxq->pool = rte_pktmbuf_pool_create(buf, nb_mbuf, MEMPOOL_CACHE_SIZE, 0,
Signature · 1 line
2.51.0
PATCH v4 4/4 examples: enable format truncation warning
In reply to Stephen HemmingerThere were several hidden bugs in examples because format truncation warning was disabled.
--- examples/meson.build | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
examples/meson.build +2 -3
@@ -78,10 +78,9 @@ else examples = get_option('examples').split(',') allow_skips = false # error out if we can't build a requested app endif + default_cflags = machine_args -if cc.has_argument('-Wno-format-truncation') - default_cflags += '-Wno-format-truncation' -endif + default_ldflags = dpdk_extra_ldflags if get_option('default_library') == 'static' and not is_windows default_ldflags += ['-Wl,--export-dynamic']
Signature · 1 line
2.51.0
Re: [PATCH v4 0/4] examples: fix format-truncation errors
In reply to Stephen HemmingerQuoted 11 lines · Stephen Hemminger
Enabling format-truncation warning in examples show several possible overflows that are easily addressed.
v4 - use
RTE_STR()Stephen Hemminger (4):
examples/server_node_efd: fix format overflow examples/vdpa: fix format overflow warning examples/ip_reassembly: add check before formatting name
examples: enable format truncation warning
Series applied.
Signature · 1 line
David Marchand
v5 cover tests: fix format truncation warnings
In reply to Stephen HemmingerEnable format truncation warnings on the tests and fix where there was an issue.
v5 - one more case of huge page possible name overflow
Stephen Hemminger (8):
test: increase size of memzone name
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: use unit test runner for eal flags
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++++- app/test/test_eal_flags.c | 174 ++++++++++++++++---------------------- app/test/test_memzone.c | 2 +- app/test/test_table_acl.c | 4 +- 6 files changed, 96 insertions(+), 115 deletions(-)
Signature · 1 line
2.51.0
PATCH v5 1/8 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v5 2/8 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/test_eal_flags.c | 126 +++++++++++++------------------------- 1 file changed, 42 insertions(+), 84 deletions(-)
app/test/test_eal_flags.c +42 -84
@@ -121,6 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" +#define file_prefix "--file-prefix" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -312,6 +314,25 @@ get_number_of_sockets(void) }#endif +static const char * +get_file_prefix (void ) +{ +#ifdef RTE_EXEC_ENV_FREEBSD +/* BSD target doesn't support prefixes at this point */ +return "" ; +#else +static char prefix[PATH_MAX +sizeof (file_prefix)]; +char tmp[PATH_MAX ]; + +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +printf ("Error - unable to get current prefix!\n" ); +return NULL ; + } +snprintf (prefix,sizeof (prefix), file_prefix"=%s" , tmp); +return prefix; +#endif +} +/* * Test that the app doesn't run with invalid allow option. * Final tests ensures it does run with valid options as sanity check (one
@@ -320,18 +341,10 @@ get_number_of_sockets(void)static int test_allow_flag (void ) { -unsigned i; -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +unsigned int i; +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *wlinval[][7 ] = { {prgname, prefix, mp_flag,
@@ -387,17 +400,9 @@ test_allow_flag(void)static int test_invalid_b_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *blinval[][5 ] = { {prgname, prefix, mp_flag,"-b" ,"error" },
@@ -491,17 +496,9 @@ test_invalid_vdev_flag(void)static int test_invalid_r_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *rinval[][5 ] = { {prgname, prefix, mp_flag,"-r" ,"error" },
@@ -535,17 +532,9 @@ test_invalid_r_flag(void)static int test_missing_c_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -c flag but no coremask value */ const char *argv1[] = { prgname, prefix, mp_flag,"-c" };
@@ -694,17 +683,9 @@ test_missing_c_flag(void)static int test_main_lcore_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char *prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif if (!rte_lcore_is_enabled (0 ) || !rte_lcore_is_enabled (1 ))return TEST_SKIPPED ;
@@ -751,17 +732,9 @@ test_main_lcore_flag(void)static int test_invalid_n_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -n flag but no value */ const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
@@ -803,18 +776,12 @@ test_invalid_n_flag(void)static int test_no_hpet_flag (void ) { -char prefix[PATH_MAX ] ="" ; -#ifdef RTE_EXEC_ENV_FREEBSD return 0 ;#else -char tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* With --no-hpet */ const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
@@ -829,6 +796,7 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" );return -1 ; } +#return 0 ; }
@@ -898,6 +866,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" ); return -1; } +#endif return 0 ; }
@@ -913,17 +882,14 @@ test_misc_flags(void)const char * prefix ="" ;const char * nosh_prefix ="" ;#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; +const char *prefix =get_file_prefix ();const char * nosh_prefix ="--file-prefix=noshconf" ;FILE * hugedir_handle =NULL ;char line[PATH_MAX ] = {0 };unsigned i, isempty =1 ; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);/* * get first valid hugepage path
@@ -1525,17 +1491,9 @@ populate_socket_mem_param(int num_sockets, const char *mem,static int test_memory_flags (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* valid -m flag and mp flag */ const char *argv0[] = {prgname, prefix, mp_flag,
Signature · 1 line
2.51.0
PATCH v5 3/8 test: avoid overflowing huge directory path
In reply to Stephen HemmingerSince the tests construct a mulit-part path in huge pages directory, avoid overflowing the string buffer by limiting the possible huge page directory. Can never happen in real life, but compiler gives warnings about string overflow that it could.
--- app/test/test_eal_flags.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
app/test/test_eal_flags.c +3 -3
@@ -172,7 +172,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action)struct dirent *dirent =NULL ;char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; +char hugedir[PATH_MAX -NAME_MAX ] = {0 };char line[PATH_MAX ] = {0 };int fd, lck_result, result =0 ;
@@ -222,7 +222,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX +NAME_MAX ] = {0 };snprintf (file_path,sizeof (file_path),"%s/%s" , hugedir, dirent->d_name);
@@ -873,7 +873,7 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; +char hugepath[PATH_MAX -NAME_MAX ] = {0 };char hugepath_dir[PATH_MAX ] = {0 };char hugepath_dir2[PATH_MAX ] = {0 };char hugepath_dir3[PATH_MAX ] = {0 };
Signature · 1 line
2.51.0
PATCH v5 4/8 test: use unit test runner for eal flags
In reply to Stephen HemmingerMake the sub tests in eal flags suite into a group so that they are not individual tests.
--- app/test/test_eal_flags.c | 46 +++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-)
app/test/test_eal_flags.c +30 -16
@@ -777,7 +777,7 @@ static inttest_no_hpet_flag (void) {#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; +return TEST_SKIPPED ;#else const char *prefix =get_file_prefix ();if (prefix ==NULL )
@@ -796,8 +796,8 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" ); return -1; } -#return 0 ; +#endif }/*
@@ -866,7 +866,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" );return -1 ; } -#endif +return 0 ; }
@@ -1208,7 +1208,7 @@ test_file_prefix(void)char prefix[PATH_MAX ] ="" ;#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; +return TEST_SKIPPED ;#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );
@@ -1650,15 +1650,29 @@ test_memory_flags(void)#endif /* !RTE_EXEC_ENV_WINDOWS */ -REGISTER_FAST_TEST (eal_flags_c_opt_autotest, false, false, test_missing_c_flag); -REGISTER_FAST_TEST (eal_flags_main_opt_autotest, false, false, test_main_lcore_flag); -REGISTER_FAST_TEST (eal_flags_n_opt_autotest, false, false, test_invalid_n_flag); -REGISTER_FAST_TEST (eal_flags_hpet_autotest, false, false, test_no_hpet_flag); -REGISTER_FAST_TEST (eal_flags_no_huge_autotest, false, false, test_no_huge_flag); -REGISTER_FAST_TEST (eal_flags_a_opt_autotest, false, false, test_allow_flag); -REGISTER_FAST_TEST (eal_flags_b_opt_autotest, false, false, test_invalid_b_flag); -REGISTER_FAST_TEST (eal_flags_vdev_opt_autotest, false, false, test_invalid_vdev_flag); -REGISTER_FAST_TEST (eal_flags_r_opt_autotest, false, false, test_invalid_r_flag); -REGISTER_FAST_TEST (eal_flags_mem_autotest, false, false, test_memory_flags); -REGISTER_FAST_TEST (eal_flags_file_prefix_autotest, false, false, test_file_prefix); -REGISTER_FAST_TEST (eal_flags_misc_autotest, false, false, test_misc_flags); +static struct unit_test_suite eal_flags_test_suite = { + .suite_name ="EAL flags unit test suite" , + .unit_test_cases = { +TEST_CASE (test_missing_c_flag), +TEST_CASE (test_main_lcore_flag), +TEST_CASE (test_invalid_n_flag), +TEST_CASE (test_no_hpet_flag), +TEST_CASE (test_no_huge_flag), +TEST_CASE (test_allow_flag), +TEST_CASE (test_invalid_b_flag), +TEST_CASE (test_invalid_vdev_flag), +TEST_CASE (test_invalid_r_flag), +TEST_CASE (test_memory_flags), +TEST_CASE (test_file_prefix), +TEST_CASE (test_misc_flags), +TEST_CASES_END () + } +}; + +static int +test_eal_flags (void ) +{ +return unit_test_suite_runner (&eal_flags_test_suite); +} + +REGISTER_FAST_TEST (eal_flags_autotest, false, false, test_eal_flags);
Signature · 1 line
2.51.0
PATCH v5 5/8 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
app/test/test_table_acl.c +2 -2
@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */
@@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int
Signature · 1 line
2.51.0
PATCH v5 6/8 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v5 7/8 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v5 8/8 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
v6 cover tests: fix format truncation warnings
In reply to Stephen HemmingerThis series fixes compiler warnings about format truncation in the unit tests.
v6 - alternate strategy for handling huge page mount point
- fix warnings which only show up in RHEL8
Stephen Hemminger (9):
test: increase size of memzone name
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: use unit test runner for eal flags
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in mp_secondary and pdump
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 283 ++++++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 4 +- app/test/test_pdump.c | 4 +- app/test/test_table_acl.c | 4 +- 8 files changed, 140 insertions(+), 188 deletions(-)
Signature · 1 line
2.51.0
PATCH v6 1/9 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v6 2/9 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/test_eal_flags.c | 126 +++++++++++++------------------------- 1 file changed, 42 insertions(+), 84 deletions(-)
app/test/test_eal_flags.c +42 -84
@@ -121,6 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" +#define file_prefix "--file-prefix" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -312,6 +314,25 @@ get_number_of_sockets(void) }#endif +static const char * +get_file_prefix (void ) +{ +#ifdef RTE_EXEC_ENV_FREEBSD +/* BSD target doesn't support prefixes at this point */ +return "" ; +#else +static char prefix[PATH_MAX +sizeof (file_prefix)]; +char tmp[PATH_MAX ]; + +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +printf ("Error - unable to get current prefix!\n" ); +return NULL ; + } +snprintf (prefix,sizeof (prefix), file_prefix"=%s" , tmp); +return prefix; +#endif +} +/* * Test that the app doesn't run with invalid allow option. * Final tests ensures it does run with valid options as sanity check (one
@@ -320,18 +341,10 @@ get_number_of_sockets(void)static int test_allow_flag (void ) { -unsigned i; -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +unsigned int i; +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *wlinval[][7 ] = { {prgname, prefix, mp_flag,
@@ -387,17 +400,9 @@ test_allow_flag(void)static int test_invalid_b_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *blinval[][5 ] = { {prgname, prefix, mp_flag,"-b" ,"error" },
@@ -491,17 +496,9 @@ test_invalid_vdev_flag(void)static int test_invalid_r_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *rinval[][5 ] = { {prgname, prefix, mp_flag,"-r" ,"error" },
@@ -535,17 +532,9 @@ test_invalid_r_flag(void)static int test_missing_c_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -c flag but no coremask value */ const char *argv1[] = { prgname, prefix, mp_flag,"-c" };
@@ -694,17 +683,9 @@ test_missing_c_flag(void)static int test_main_lcore_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char *prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif if (!rte_lcore_is_enabled (0 ) || !rte_lcore_is_enabled (1 ))return TEST_SKIPPED ;
@@ -751,17 +732,9 @@ test_main_lcore_flag(void)static int test_invalid_n_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -n flag but no value */ const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
@@ -803,18 +776,12 @@ test_invalid_n_flag(void)static int test_no_hpet_flag (void ) { -char prefix[PATH_MAX ] ="" ; -#ifdef RTE_EXEC_ENV_FREEBSD return 0 ;#else -char tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* With --no-hpet */ const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
@@ -829,6 +796,7 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" );return -1 ; } +#return 0 ; }
@@ -898,6 +866,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" ); return -1; } +#endif return 0 ; }
@@ -913,17 +882,14 @@ test_misc_flags(void)const char * prefix ="" ;const char * nosh_prefix ="" ;#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; +const char *prefix =get_file_prefix ();const char * nosh_prefix ="--file-prefix=noshconf" ;FILE * hugedir_handle =NULL ;char line[PATH_MAX ] = {0 };unsigned i, isempty =1 ; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);/* * get first valid hugepage path
@@ -1525,17 +1491,9 @@ populate_socket_mem_param(int num_sockets, const char *mem,static int test_memory_flags (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =get_file_prefix (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* valid -m flag and mp flag */ const char *argv0[] = {prgname, prefix, mp_flag,
Signature · 1 line
2.51.0
PATCH v6 3/9 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
--- app/test/test_eal_flags.c | 115 ++++++++++++++------------------------ 1 file changed, 43 insertions(+), 72 deletions(-)
app/test/test_eal_flags.c +43 -72
@@ -107,6 +107,7 @@ test_misc_flags(void)#include <sys/wait.h> #include <limits.h> #include <fcntl.h> +#include <mntent.h> #include <rte_lcore.h> #include <rte_debug.h>
@@ -123,6 +124,8 @@ test_misc_flags(void)#define vdev "--vdev"#define file_prefix "--file-prefix" +#define FS_HUGETLB "hugetlbfs" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -136,24 +139,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,45 +150,39 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error creating hugefile filename prefix\n" );return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error parsing /proc/mounts!\n" );return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); -printf ("Error reading %s: %s\n" , hugedir,strerror (errno)); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts); +printf ("Error reading %s: %s\n" , entry->mnt_dir,strerror (errno));return -1 ; }
@@ -222,10 +201,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -240,6 +219,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -253,10 +234,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -278,7 +256,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; }
@@ -873,10 +851,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -884,9 +862,7 @@ test_misc_flags(void)#else const char *prefix =get_file_prefix ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL )return -1 ;
@@ -896,29 +872,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error opening /proc/mounts!\n" );return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
Signature · 1 line
2.51.0
PATCH v6 4/9 test: use unit test runner for eal flags
In reply to Stephen HemmingerMake the sub tests in eal flags suite into a group so that they are not individual tests.
--- app/test/test_eal_flags.c | 46 +++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-)
app/test/test_eal_flags.c +30 -16
@@ -755,7 +755,7 @@ static inttest_no_hpet_flag (void) {#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; +return TEST_SKIPPED ;#else const char *prefix =get_file_prefix ();if (prefix ==NULL )
@@ -774,8 +774,8 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" ); return -1; } -#return 0 ; +#endif }/*
@@ -844,7 +844,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" );return -1 ; } -#endif +return 0 ; }
@@ -1179,7 +1179,7 @@ test_file_prefix(void)char prefix[PATH_MAX ] ="" ;#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; +return TEST_SKIPPED ;#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );
@@ -1621,15 +1621,29 @@ test_memory_flags(void)#endif /* !RTE_EXEC_ENV_WINDOWS */ -REGISTER_FAST_TEST (eal_flags_c_opt_autotest, false, false, test_missing_c_flag); -REGISTER_FAST_TEST (eal_flags_main_opt_autotest, false, false, test_main_lcore_flag); -REGISTER_FAST_TEST (eal_flags_n_opt_autotest, false, false, test_invalid_n_flag); -REGISTER_FAST_TEST (eal_flags_hpet_autotest, false, false, test_no_hpet_flag); -REGISTER_FAST_TEST (eal_flags_no_huge_autotest, false, false, test_no_huge_flag); -REGISTER_FAST_TEST (eal_flags_a_opt_autotest, false, false, test_allow_flag); -REGISTER_FAST_TEST (eal_flags_b_opt_autotest, false, false, test_invalid_b_flag); -REGISTER_FAST_TEST (eal_flags_vdev_opt_autotest, false, false, test_invalid_vdev_flag); -REGISTER_FAST_TEST (eal_flags_r_opt_autotest, false, false, test_invalid_r_flag); -REGISTER_FAST_TEST (eal_flags_mem_autotest, false, false, test_memory_flags); -REGISTER_FAST_TEST (eal_flags_file_prefix_autotest, false, false, test_file_prefix); -REGISTER_FAST_TEST (eal_flags_misc_autotest, false, false, test_misc_flags); +static struct unit_test_suite eal_flags_test_suite = { + .suite_name ="EAL flags unit test suite" , + .unit_test_cases = { +TEST_CASE (test_missing_c_flag), +TEST_CASE (test_main_lcore_flag), +TEST_CASE (test_invalid_n_flag), +TEST_CASE (test_no_hpet_flag), +TEST_CASE (test_no_huge_flag), +TEST_CASE (test_allow_flag), +TEST_CASE (test_invalid_b_flag), +TEST_CASE (test_invalid_vdev_flag), +TEST_CASE (test_invalid_r_flag), +TEST_CASE (test_memory_flags), +TEST_CASE (test_file_prefix), +TEST_CASE (test_misc_flags), +TEST_CASES_END () + } +}; + +static int +test_eal_flags (void ) +{ +return unit_test_suite_runner (&eal_flags_test_suite); +} + +REGISTER_FAST_TEST (eal_flags_autotest, false, false, test_eal_flags);
Signature · 1 line
2.51.0
PATCH v6 5/9 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
app/test/test_table_acl.c +2 -2
@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */
@@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int
Signature · 1 line
2.51.0
PATCH v6 6/9 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v6 7/9 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v6 8/9 test: fix overflow warnings in mp_secondary and pdump
In reply to Stephen HemmingerBoth these tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by allowing for bigger string.
--- app/test/test_mp_secondary.c | 4 ++-- app/test/test_pdump.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
app/test/test_mp_secondary.c +2 -2
@@ -69,8 +69,8 @@ run_secondary_instances(void)char coremask[10 ];#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +char tmp[PATH_MAX ]; +char prefix[PATH_MAX +sizeof ("--file-prefix=" )];get_current_prefix (tmp,sizeof (tmp));
app/test/test_pdump.c +2 -2
@@ -176,8 +176,8 @@ run_pdump_server_tests(void)char coremask[10 ];#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +char tmp[PATH_MAX ]; +char prefix[PATH_MAX +sizeof ("--file-prefix" )];get_current_prefix (tmp,sizeof (tmp));snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);
Signature · 1 line
2.51.0
PATCH v6 9/9 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
Re: [PATCH v6 0/9] tests: fix format truncation warnings
In reply to Stephen HemmingerQuoted 27 lines · Stephen Hemminger
This series fixes compiler warnings about format truncation in the unit tests.
v6 - alternate strategy for handling huge page mount point
- fix warnings which only show up in RHEL8Stephen Hemminger (9):
test: increase size of memzone name
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: use unit test runner for eal flags
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings inmp_secondaryand pdump
test: re-enable format-truncation warningsapp/test/meson.build | 1 - app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 283 ++++++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 4 +- app/test/test_pdump.c | 4 +- app/test/test_table_acl.c | 4 +- 8 files changed, 140 insertions(+), 188 deletions(-)
Re: [PATCH v6 0/9] tests: fix format truncation warnings
In reply to Stephen HemmingerHi. There are some DTS failures on this testbed due to misconfiguration applied by us at UNH last week. I'm fixing it and rerunning testing which will update the CI checks for your patchseries.
v7 cover test: fix format truncation warnings
In reply to Stephen HemmingerThis series fixes compiler warnings in unit tests about format truncation.
v7 - refactor to have common code for formatting --file-prefix=XXX
which addresses a couple more warnings produced on RHEL8.
Stephen Hemminger (10):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in several places by using common code
test: use unit test runner for eal flags
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/process.h | 38 ++++- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 264 ++++++++++++-------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 14 +- app/test/test_pdump.c | 13 +- app/test/test_table_acl.c | 4 +- app/test/test_timer_secondary.c | 12 +- 10 files changed, 161 insertions(+), 217 deletions(-)
Signature · 1 line
2.51.0
PATCH v7 1/10 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v7 2/10 test: add new function to get current file-prefix arg
In reply to Stephen HemmingerRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is NAME_MAX (128) not PATH_MAX(4096) so save a significant amount of space.
--- app/test/process.h | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)
app/test/process.h +32 -6
@@ -199,15 +199,22 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no arguement passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg(void) +{ + /* BSD target doesn't support prefixes at this point */ + return ""; +} +#else + #ifdef RTE_EXEC_ENV_LINUX static inline char * get_current_prefix(char *prefix, int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; + char path[64]; + char buf[PATH_MAX]; /* get file for config (fd is always 3) */ snprintf(path, sizeof(path), "/proc/self/fd/%d", 3);
@@ -216,10 +223,29 @@ get_current_prefix(char *prefix, int size)if (readlink (path, buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config" +* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif
Signature · 1 line
2.51.0
PATCH v7 3/10 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/test_eal_flags.c | 107 ++++++++------------------------------ 1 file changed, 23 insertions(+), 84 deletions(-)
app/test/test_eal_flags.c +23 -84
@@ -121,6 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" +#define file_prefix "--file-prefix" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -320,18 +322,10 @@ get_number_of_sockets(void)static int test_allow_flag (void ) { -unsigned i; -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +unsigned int i; +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *wlinval[][7 ] = { {prgname, prefix, mp_flag,
@@ -387,17 +381,9 @@ test_allow_flag(void)static int test_invalid_b_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *blinval[][5 ] = { {prgname, prefix, mp_flag,"-b" ,"error" },
@@ -491,17 +477,9 @@ test_invalid_vdev_flag(void)static int test_invalid_r_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *rinval[][5 ] = { {prgname, prefix, mp_flag,"-r" ,"error" },
@@ -535,17 +513,9 @@ test_invalid_r_flag(void)static int test_missing_c_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -c flag but no coremask value */ const char *argv1[] = { prgname, prefix, mp_flag,"-c" };
@@ -694,17 +664,9 @@ test_missing_c_flag(void)static int test_main_lcore_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char *prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif if (!rte_lcore_is_enabled (0 ) || !rte_lcore_is_enabled (1 ))return TEST_SKIPPED ;
@@ -751,17 +713,9 @@ test_main_lcore_flag(void)static int test_invalid_n_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -n flag but no value */ const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
@@ -803,18 +757,12 @@ test_invalid_n_flag(void)static int test_no_hpet_flag (void ) { -char prefix[PATH_MAX ] ="" ; -#ifdef RTE_EXEC_ENV_FREEBSD return 0 ;#else -char tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* With --no-hpet */ const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
@@ -829,6 +777,7 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" );return -1 ; } +#return 0 ; }
@@ -898,6 +847,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" ); return -1; } +#endif return 0 ; }
@@ -913,17 +863,14 @@ test_misc_flags(void)const char * prefix ="" ;const char * nosh_prefix ="" ;#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; +const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ;FILE * hugedir_handle =NULL ;char line[PATH_MAX ] = {0 };unsigned i, isempty =1 ; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);/* * get first valid hugepage path
@@ -1525,17 +1472,9 @@ populate_socket_mem_param(int num_sockets, const char *mem,static int test_memory_flags (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* valid -m flag and mp flag */ const char *argv0[] = {prgname, prefix, mp_flag,
Signature · 1 line
2.51.0
PATCH v7 4/10 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
--- app/test/test_eal_flags.c | 115 ++++++++++++++------------------------ 1 file changed, 43 insertions(+), 72 deletions(-)
app/test/test_eal_flags.c +43 -72
@@ -107,6 +107,7 @@ test_misc_flags(void)#include <sys/wait.h> #include <limits.h> #include <fcntl.h> +#include <mntent.h> #include <rte_lcore.h> #include <rte_debug.h>
@@ -123,6 +124,8 @@ test_misc_flags(void)#define vdev "--vdev"#define file_prefix "--file-prefix" +#define FS_HUGETLB "hugetlbfs" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -136,24 +139,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,45 +150,39 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error creating hugefile filename prefix\n" );return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error parsing /proc/mounts!\n" );return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); -printf ("Error reading %s: %s\n" , hugedir,strerror (errno)); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts); +printf ("Error reading %s: %s\n" , entry->mnt_dir,strerror (errno));return -1 ; }
@@ -222,10 +201,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -240,6 +219,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -253,10 +234,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -278,7 +256,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; }
@@ -854,10 +832,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -865,9 +843,7 @@ test_misc_flags(void)#else const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL )return -1 ;
@@ -877,29 +853,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error opening /proc/mounts!\n" );return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
Signature · 1 line
2.51.0
PATCH v7 5/10 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
app/test/test_table_acl.c +2 -2
@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */
@@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int
Signature · 1 line
2.51.0
PATCH v7 6/10 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v7 7/10 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v7 8/10 test: fix overflow warnings in several places by using common code
In reply to Stephen HemmingerSeveral tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by allowing for bigger string.
--- app/test/test_mp_secondary.c | 14 ++++---------- app/test/test_pdump.c | 13 ++++--------- app/test/test_timer_secondary.c | 12 ++++-------- 3 files changed, 12 insertions(+), 27 deletions(-)
app/test/test_mp_secondary.c +4 -10
@@ -67,17 +67,11 @@ run_secondary_instances(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); - -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *argv1[] = {
app/test/test_pdump.c +4 -9
@@ -174,16 +174,11 @@ run_pdump_server_tests(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *const argv1[] = {
app/test/test_timer_secondary.c +4 -8
@@ -51,16 +51,12 @@ static int timer_secondary_spawn_wait(unsigned int lcore) {char coremask[10 ]; -#ifdef RTE_EXEC_ENV_LINUXAPP -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +const char *prefix; -get_current_prefix (tmp,sizeof (tmp)); + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ; -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif char const *argv[] = { prgname,"-c" , coremask,
Signature · 1 line
2.51.0
PATCH v7 9/10 test: use unit test runner for eal flags
In reply to Stephen HemmingerMake the sub tests in eal flags suite into a group so that they are not individual tests.
--- app/test/test_eal_flags.c | 46 +++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-)
app/test/test_eal_flags.c +30 -16
@@ -736,7 +736,7 @@ static inttest_no_hpet_flag (void) {#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; +return TEST_SKIPPED ;#else const char *prefix =file_prefix_arg ();if (prefix ==NULL )
@@ -755,8 +755,8 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" ); return -1; } -#return 0 ; +#endif }/*
@@ -825,7 +825,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" );return -1 ; } -#endif +return 0 ; }
@@ -1160,7 +1160,7 @@ test_file_prefix(void)char prefix[PATH_MAX ] ="" ;#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; +return TEST_SKIPPED ;#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );
@@ -1602,15 +1602,29 @@ test_memory_flags(void)#endif /* !RTE_EXEC_ENV_WINDOWS */ -REGISTER_FAST_TEST (eal_flags_c_opt_autotest, false, false, test_missing_c_flag); -REGISTER_FAST_TEST (eal_flags_main_opt_autotest, false, false, test_main_lcore_flag); -REGISTER_FAST_TEST (eal_flags_n_opt_autotest, false, false, test_invalid_n_flag); -REGISTER_FAST_TEST (eal_flags_hpet_autotest, false, false, test_no_hpet_flag); -REGISTER_FAST_TEST (eal_flags_no_huge_autotest, false, false, test_no_huge_flag); -REGISTER_FAST_TEST (eal_flags_a_opt_autotest, false, false, test_allow_flag); -REGISTER_FAST_TEST (eal_flags_b_opt_autotest, false, false, test_invalid_b_flag); -REGISTER_FAST_TEST (eal_flags_vdev_opt_autotest, false, false, test_invalid_vdev_flag); -REGISTER_FAST_TEST (eal_flags_r_opt_autotest, false, false, test_invalid_r_flag); -REGISTER_FAST_TEST (eal_flags_mem_autotest, false, false, test_memory_flags); -REGISTER_FAST_TEST (eal_flags_file_prefix_autotest, false, false, test_file_prefix); -REGISTER_FAST_TEST (eal_flags_misc_autotest, false, false, test_misc_flags); +static struct unit_test_suite eal_flags_test_suite = { + .suite_name ="EAL flags unit test suite" , + .unit_test_cases = { +TEST_CASE (test_missing_c_flag), +TEST_CASE (test_main_lcore_flag), +TEST_CASE (test_invalid_n_flag), +TEST_CASE (test_no_hpet_flag), +TEST_CASE (test_no_huge_flag), +TEST_CASE (test_allow_flag), +TEST_CASE (test_invalid_b_flag), +TEST_CASE (test_invalid_vdev_flag), +TEST_CASE (test_invalid_r_flag), +TEST_CASE (test_memory_flags), +TEST_CASE (test_file_prefix), +TEST_CASE (test_misc_flags), +TEST_CASES_END () + } +}; + +static int +test_eal_flags (void ) +{ +return unit_test_suite_runner (&eal_flags_test_suite); +} + +REGISTER_FAST_TEST (eal_flags_autotest, false, false, test_eal_flags);
Signature · 1 line
2.51.0
PATCH v7 10/10 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
v8 cover test: cleanup and format truncation fixes
In reply to Stephen HemmingerThis series fixes compiler warnings in unit tests about format truncation. It also refactor some the related tests to be cleaner and use standard API's.
v8 - avoid building unused code on FreeBSD
Stephen Hemminger (10):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in several places by using common code
test: use unit test runner for eal flags
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/process.h | 38 ++++- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 294 +++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 14 +- app/test/test_pdump.c | 13 +- app/test/test_table_acl.c | 4 +- app/test/test_timer_secondary.c | 12 +- 10 files changed, 170 insertions(+), 238 deletions(-)
Signature · 1 line
2.51.0
PATCH v8 1/10 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v8 2/10 test: add new function to get current file-prefix arg
In reply to Stephen HemmingerRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is NAME_MAX (128) not PATH_MAX(4096) so save a significant amount of space.
--- app/test/process.h | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)
app/test/process.h +32 -6
@@ -199,15 +199,22 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no argument passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg(void) +{ + /* BSD target doesn't support prefixes at this point */ + return ""; +} +#else + #ifdef RTE_EXEC_ENV_LINUX static inline char * get_current_prefix(char *prefix, int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; + char path[64]; + char buf[PATH_MAX]; /* get file for config (fd is always 3) */ snprintf(path, sizeof(path), "/proc/self/fd/%d", 3);
@@ -216,10 +223,29 @@ get_current_prefix(char *prefix, int size)if (readlink (path, buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config" +* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif
Signature · 1 line
2.51.0
PATCH v8 3/10 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/test_eal_flags.c | 107 ++++++++------------------------------ 1 file changed, 23 insertions(+), 84 deletions(-)
app/test/test_eal_flags.c +23 -84
@@ -121,6 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" +#define file_prefix "--file-prefix" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"
@@ -320,18 +322,10 @@ get_number_of_sockets(void)static int test_allow_flag (void ) { -unsigned i; -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +unsigned int i; +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *wlinval[][7 ] = { {prgname, prefix, mp_flag,
@@ -387,17 +381,9 @@ test_allow_flag(void)static int test_invalid_b_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *blinval[][5 ] = { {prgname, prefix, mp_flag,"-b" ,"error" },
@@ -491,17 +477,9 @@ test_invalid_vdev_flag(void)static int test_invalid_r_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif const char *rinval[][5 ] = { {prgname, prefix, mp_flag,"-r" ,"error" },
@@ -535,17 +513,9 @@ test_invalid_r_flag(void)static int test_missing_c_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -c flag but no coremask value */ const char *argv1[] = { prgname, prefix, mp_flag,"-c" };
@@ -694,17 +664,9 @@ test_missing_c_flag(void)static int test_main_lcore_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char *prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif if (!rte_lcore_is_enabled (0 ) || !rte_lcore_is_enabled (1 ))return TEST_SKIPPED ;
@@ -751,17 +713,9 @@ test_main_lcore_flag(void)static int test_invalid_n_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* -n flag but no value */ const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
@@ -803,18 +757,12 @@ test_invalid_n_flag(void)static int test_no_hpet_flag (void ) { -char prefix[PATH_MAX ] ="" ; -#ifdef RTE_EXEC_ENV_FREEBSD return 0 ;#else -char tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* With --no-hpet */ const char *argv1[] = {prgname, prefix, mp_flag, no_hpet};
@@ -829,6 +777,7 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" );return -1 ; } +#return 0 ; }
@@ -898,6 +847,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" ); return -1; } +#endif return 0 ; }
@@ -913,17 +863,14 @@ test_misc_flags(void)const char * prefix ="" ;const char * nosh_prefix ="" ;#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; +const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ;FILE * hugedir_handle =NULL ;char line[PATH_MAX ] = {0 };unsigned i, isempty =1 ; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);/* * get first valid hugepage path
@@ -1525,17 +1472,9 @@ populate_socket_mem_param(int num_sockets, const char *mem,static int test_memory_flags (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -/* BSD target doesn't support prefixes at this point */ -const char * prefix ="" ; -#else -char prefix[PATH_MAX ], tmp[PATH_MAX ]; -if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { -printf ("Error - unable to get current prefix!\n" ); +const char *prefix =file_prefix_arg (); +if (prefix ==NULL )return -1 ; - } -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#endif /* valid -m flag and mp flag */ const char *argv0[] = {prgname, prefix, mp_flag,
Signature · 1 line
2.51.0
PATCH v8 4/10 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
Hugepages aren't used on Window or FreeBSD so this is skipped there.
--- app/test/test_eal_flags.c | 136 ++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 78 deletions(-)
app/test/test_eal_flags.c +58 -78
@@ -123,12 +123,17 @@ test_misc_flags(void)#define vdev "--vdev"#define file_prefix "--file-prefix" +#define FS_HUGETLB "hugetlbfs" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"#define SOCKET_MEM_STRLEN (RTE_MAX_NUMA_NODES * 20)#define launch_proc (ARGV ) process_dup(ARGV, RTE_DIM(ARGV), __func__) +#ifdef RTE_EXEC_ENV_LINUX +#include <mntent.h> +enum hugepage_action {HUGEPAGE_CHECK_EXISTS =0 , HUGEPAGE_CHECK_LOCKED,
@@ -136,24 +141,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,45 +152,39 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error creating hugefile filename prefix\n" );return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error parsing /proc/mounts!\n" );return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); -printf ("Error reading %s: %s\n" , hugedir,strerror (errno)); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts); +printf ("Error reading %s: %s\n" , entry->mnt_dir,strerror (errno));return -1 ; }
@@ -222,10 +203,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -240,6 +221,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -253,10 +236,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -278,11 +258,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; } -#ifdef RTE_EXEC_ENV_LINUX /* * count the number of "node*" files in /sys/devices/system/node/ */
@@ -854,10 +833,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -865,9 +844,7 @@ test_misc_flags(void)#else const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL )return -1 ;
@@ -877,29 +854,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error opening /proc/mounts!\n" );return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
@@ -1168,6 +1140,17 @@ test_misc_flags(void)return -1 ; } +#ifdef RTE_EXEC_ENV_FREEBSD + +static int +test_file_prefix (void ) +{ +printf ("file_prefix not supported on FreeBSD, skipping test\n" ); +return TEST_SKIPPED ; +} + +#else +static int test_file_prefix (void ) {
@@ -1186,16 +1169,12 @@ test_file_prefix(void)* run in legacy mode, and not present at all after run in default * mem mode */ -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX ]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );return -1 ; } -#endif /* this should fail unless the test itself is run with "memtest" prefix */ const char *argv0[] = {prgname, mp_flag,"-m" ,
@@ -1431,6 +1410,7 @@ test_file_prefix(void) return 0; } +#endif /* This function writes in passed buf pointer a valid --socket-mem= option * for num_sockets then concatenates the provided suffix string.
Signature · 1 line
2.51.0
PATCH v8 5/10 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
app/test/test_table_acl.c +2 -2
@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */
@@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int
Signature · 1 line
2.51.0
PATCH v8 6/10 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v8 7/10 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v8 8/10 test: fix overflow warnings in several places by using common code
In reply to Stephen HemmingerSeveral tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by allowing for bigger string.
--- app/test/test_mp_secondary.c | 14 ++++---------- app/test/test_pdump.c | 13 ++++--------- app/test/test_timer_secondary.c | 12 ++++-------- 3 files changed, 12 insertions(+), 27 deletions(-)
app/test/test_mp_secondary.c +4 -10
@@ -67,17 +67,11 @@ run_secondary_instances(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); - -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *argv1[] = {
app/test/test_pdump.c +4 -9
@@ -174,16 +174,11 @@ run_pdump_server_tests(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *const argv1[] = {
app/test/test_timer_secondary.c +4 -8
@@ -51,16 +51,12 @@ static int timer_secondary_spawn_wait(unsigned int lcore) {char coremask[10 ]; -#ifdef RTE_EXEC_ENV_LINUXAPP -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +const char *prefix; -get_current_prefix (tmp,sizeof (tmp)); + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ; -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif char const *argv[] = { prgname,"-c" , coremask,
Signature · 1 line
2.51.0
PATCH v8 9/10 test: use unit test runner for eal flags
In reply to Stephen HemmingerMake the sub tests in eal flags suite into a group so that they are not individual tests.
--- app/test/test_eal_flags.c | 75 ++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 41 deletions(-)
app/test/test_eal_flags.c +34 -41
@@ -32,13 +32,6 @@ test_invalid_n_flag(void)return TEST_SKIPPED ; } -static int -test_no_hpet_flag (void ) -{ -printf ("no_hpet_flag not supported on Windows, skipping test\n" ); -return TEST_SKIPPED ; -} -static int test_no_huge_flag (void ) {
@@ -81,13 +74,6 @@ test_memory_flags(void)return TEST_SKIPPED ; } -static int -test_file_prefix (void ) -{ -printf ("file_prefix not supported on Windows, skipping test\n" ); -return TEST_SKIPPED ; -} -static int test_misc_flags (void ) {
@@ -730,15 +716,13 @@ test_invalid_n_flag(void)return 0 ; } +#ifdef RTE_EXEC_ENV_LINUX /* * Test that the app runs with HPET, and without HPET */ static int test_no_hpet_flag (void ) { -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else const char *prefix =file_prefix_arg ();if (prefix ==NULL )return -1 ;
@@ -756,9 +740,9 @@ test_no_hpet_flag(void)printf ("Error - process did not run ok without --no-hpet flag\n" ); return -1; } -#return 0 ; } +#endif /* * Test that the app runs with --no-huge and doesn't run when --socket-mem are
@@ -826,7 +810,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" );return -1 ; } -#endif +return 0 ; }
@@ -1140,16 +1124,7 @@ test_misc_flags(void)return -1 ; } -#ifdef RTE_EXEC_ENV_FREEBSD - -static int -test_file_prefix (void ) -{ -printf ("file_prefix not supported on FreeBSD, skipping test\n" ); -return TEST_SKIPPED ; -} - -#else +#ifdef RTE_EXEC_ENV_LINUX static int test_file_prefix (void )
@@ -1611,15 +1586,33 @@ test_memory_flags(void)#endif /* !RTE_EXEC_ENV_WINDOWS */ -REGISTER_FAST_TEST (eal_flags_c_opt_autotest, false, false, test_missing_c_flag); -REGISTER_FAST_TEST (eal_flags_main_opt_autotest, false, false, test_main_lcore_flag); -REGISTER_FAST_TEST (eal_flags_n_opt_autotest, false, false, test_invalid_n_flag); -REGISTER_FAST_TEST (eal_flags_hpet_autotest, false, false, test_no_hpet_flag); -REGISTER_FAST_TEST (eal_flags_no_huge_autotest, false, false, test_no_huge_flag); -REGISTER_FAST_TEST (eal_flags_a_opt_autotest, false, false, test_allow_flag); -REGISTER_FAST_TEST (eal_flags_b_opt_autotest, false, false, test_invalid_b_flag); -REGISTER_FAST_TEST (eal_flags_vdev_opt_autotest, false, false, test_invalid_vdev_flag); -REGISTER_FAST_TEST (eal_flags_r_opt_autotest, false, false, test_invalid_r_flag); -REGISTER_FAST_TEST (eal_flags_mem_autotest, false, false, test_memory_flags); -REGISTER_FAST_TEST (eal_flags_file_prefix_autotest, false, false, test_file_prefix); -REGISTER_FAST_TEST (eal_flags_misc_autotest, false, false, test_misc_flags); +static struct unit_test_suite eal_flags_test_suite = { + .suite_name ="EAL flags unit test suite" , + .unit_test_cases = { +TEST_CASE (test_missing_c_flag), +TEST_CASE (test_main_lcore_flag), +TEST_CASE (test_invalid_n_flag), +#ifdef RTE_EXEC_ENV_LINUX +TEST_CASE (test_no_hpet_flag), +#endif +TEST_CASE (test_no_huge_flag), +TEST_CASE (test_allow_flag), +TEST_CASE (test_invalid_b_flag), +TEST_CASE (test_invalid_vdev_flag), +TEST_CASE (test_invalid_r_flag), +TEST_CASE (test_memory_flags), +#ifdef RTE_EXEC_ENV_LINUX +TEST_CASE (test_file_prefix), +#endif +TEST_CASE (test_misc_flags), +TEST_CASES_END () + } +}; + +static int +test_eal_flags (void ) +{ +return unit_test_suite_runner (&eal_flags_test_suite); +} + +REGISTER_FAST_TEST (eal_flags_autotest, false, false, test_eal_flags);
Signature · 1 line
2.51.0
PATCH v8 10/10 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
v9 cover test: cleanup and format truncation fixes
In reply to Stephen HemmingerThis series fixes compiler warnings in unit tests about format truncation. It also refactor some the related tests to be cleaner and use standard API's.
v9 - drop using unit test runner
tests need the additional cleanup between tests
Stephen Hemminger (9):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in several places by using common code
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/process.h | 44 ++++-- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 243 +++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 14 +- app/test/test_pdump.c | 13 +- app/test/test_table_acl.c | 4 +- app/test/test_timer_secondary.c | 12 +- 10 files changed, 149 insertions(+), 214 deletions(-)
Signature · 1 line
2.51.0
PATCH v9 1/9 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v9 2/9 test: add new function to get current file-prefix arg
In reply to Stephen HemmingerRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is NAME_MAX (128) not PATH_MAX(4096) so save a significant amount of space.
--- app/test/process.h | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)
app/test/process.h +32 -6
@@ -199,15 +199,22 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no argument passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg(void) +{ + /* BSD target doesn't support prefixes at this point */ + return ""; +} +#else + #ifdef RTE_EXEC_ENV_LINUX static inline char * get_current_prefix(char *prefix, int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; + char path[64]; + char buf[PATH_MAX]; /* get file for config (fd is always 3) */ snprintf(path, sizeof(path), "/proc/self/fd/%d", 3);
@@ -216,10 +223,29 @@ get_current_prefix(char *prefix, int size)if (readlink (path, buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config" +* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif
Signature · 1 line
2.51.0
PATCH v9 3/9 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/process.h | 8 +-- app/test/test_eal_flags.c | 107 ++++++++------------------------------ 2 files changed, 25 insertions(+), 90 deletions(-)
app/test/process.h +2 -6
@@ -213,14 +213,10 @@ file_prefix_arg(void)static inline char *get_current_prefix (char *prefix,int size) { -char path[64 ];char buf[PATH_MAX ]; -/* get file for config (fd is always 3) */ -snprintf (path,sizeof (path),"/proc/self/fd/%d" ,3 ); - -/* return NULL on error */ -if (readlink (path, buf,sizeof (buf)) ==-1 ) +/* get file for config (fd is always 3) return NULL on error */ +if (readlink ("/proc/self/fd/3" , buf,sizeof (buf)) ==-1 )return NULL ;/*
app/test/test_eal_flags.c +23 -84
196 lines not on this page · raw message
Signature · 1 line
2.51.0
PATCH v9 4/9 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
Hugepages aren't used on Window or FreeBSD so this is skipped there.
--- app/test/test_eal_flags.c | 136 ++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 78 deletions(-)
app/test/test_eal_flags.c +58 -78
@@ -123,12 +123,17 @@ test_misc_flags(void)#define vdev "--vdev"#define file_prefix "--file-prefix" +#define FS_HUGETLB "hugetlbfs" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"#define SOCKET_MEM_STRLEN (RTE_MAX_NUMA_NODES * 20)#define launch_proc (ARGV ) process_dup(ARGV, RTE_DIM(ARGV), __func__) +#ifdef RTE_EXEC_ENV_LINUX +#include <mntent.h> +enum hugepage_action {HUGEPAGE_CHECK_EXISTS =0 , HUGEPAGE_CHECK_LOCKED,
@@ -136,24 +141,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,45 +152,39 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error creating hugefile filename prefix\n" );return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error parsing /proc/mounts!\n" );return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); -printf ("Error reading %s: %s\n" , hugedir,strerror (errno)); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts); +printf ("Error reading %s: %s\n" , entry->mnt_dir,strerror (errno));return -1 ; }
@@ -222,10 +203,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -240,6 +221,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -253,10 +236,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -278,11 +258,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; } -#ifdef RTE_EXEC_ENV_LINUX /* * count the number of "node*" files in /sys/devices/system/node/ */
@@ -854,10 +833,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -865,9 +844,7 @@ test_misc_flags(void)#else const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL )return -1 ;
@@ -877,29 +854,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error opening /proc/mounts!\n" );return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
@@ -1168,6 +1140,17 @@ test_misc_flags(void)return -1 ; } +#ifdef RTE_EXEC_ENV_FREEBSD + +static int +test_file_prefix (void ) +{ +printf ("file_prefix not supported on FreeBSD, skipping test\n" ); +return TEST_SKIPPED ; +} + +#else +static int test_file_prefix (void ) {
@@ -1186,16 +1169,12 @@ test_file_prefix(void)* run in legacy mode, and not present at all after run in default * mem mode */ -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX ]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );return -1 ; } -#endif /* this should fail unless the test itself is run with "memtest" prefix */ const char *argv0[] = {prgname, mp_flag,"-m" ,
@@ -1431,6 +1410,7 @@ test_file_prefix(void) return 0; } +#endif /* This function writes in passed buf pointer a valid --socket-mem= option * for num_sockets then concatenates the provided suffix string.
Signature · 1 line
2.51.0
PATCH v9 5/9 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
app/test/test_table_acl.c +2 -2
@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */
@@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int
Signature · 1 line
2.51.0
PATCH v9 6/9 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v9 7/9 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v9 8/9 test: fix overflow warnings in several places by using common code
In reply to Stephen HemmingerSeveral tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by allowing for bigger string.
--- app/test/test_mp_secondary.c | 14 ++++---------- app/test/test_pdump.c | 13 ++++--------- app/test/test_timer_secondary.c | 12 ++++-------- 3 files changed, 12 insertions(+), 27 deletions(-)
app/test/test_mp_secondary.c +4 -10
@@ -67,17 +67,11 @@ run_secondary_instances(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); - -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *argv1[] = {
app/test/test_pdump.c +4 -9
@@ -174,16 +174,11 @@ run_pdump_server_tests(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *const argv1[] = {
app/test/test_timer_secondary.c +4 -8
@@ -51,16 +51,12 @@ static int timer_secondary_spawn_wait(unsigned int lcore) {char coremask[10 ]; -#ifdef RTE_EXEC_ENV_LINUXAPP -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +const char *prefix; -get_current_prefix (tmp,sizeof (tmp)); + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ; -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif char const *argv[] = { prgname,"-c" , coremask,
Signature · 1 line
2.51.0
PATCH v9 9/9 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
v10 cover tests: enable and fix format overflow
In reply to Stephen HemmingerThis series fixes compiler warnings in unit tests about format truncation. It also refactor some the related tests to be cleaner and use standard API's.
v10 - fix on FreeBSD
- rebase for 26.03
Stephen Hemminger (9):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: refactor file prefix arg handling
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in common code
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/process.h | 44 ++++-- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 268 +++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 14 +- app/test/test_pdump.c | 13 +- app/test/test_table_acl.c | 4 +- app/test/test_timer_secondary.c | 12 +- 10 files changed, 161 insertions(+), 227 deletions(-)
Signature · 1 line
2.51.0
PATCH v10 1/9 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v10 2/9 test: add new function to get current file-prefix arg
In reply to Stephen HemmingerRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is NAME_MAX (128) not PATH_MAX(4096) so save a significant amount of space.
--- app/test/process.h | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)
app/test/process.h +32 -6
@@ -199,15 +199,22 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no argument passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg(void) +{ + /* BSD target doesn't support prefixes at this point */ + return ""; +} +#else + #ifdef RTE_EXEC_ENV_LINUX static inline char * get_current_prefix(char *prefix, int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; + char path[64]; + char buf[PATH_MAX]; /* get file for config (fd is always 3) */ snprintf(path, sizeof(path), "/proc/self/fd/%d", 3);
@@ -216,10 +223,29 @@ get_current_prefix(char *prefix, int size)if (readlink (path, buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config" +* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif
Signature · 1 line
2.51.0
RE: [PATCH v10 2/9] test: add new function to get current file-prefix arg
Quoted 42 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>
Subject: [PATCH v10 2/9] test: add new function to get current file-prefix argRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is
NAME_MAX(128) notPATH_MAX(4096)so save a significant amount of space.Signed-off-by Stephen Hemminger <[email protected]>--- app/test/process.h | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)app/test/process.h +10
-3@@ -199,15 +199,22 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no argument passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg (void ) +{ +/* BSD target doesn't support prefixes at this point */ +return "" ; +} +#else +#ifdef RTE_EXEC_ENV_LINUX
app/test/process.h process_dup(const char *const argv[], int numargs, const char *env_value) · line 220
Quoted 10 lines
app/test/process.h +2
-2static inline char *get_current_prefix (char *prefix,int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; +char path[64 ]; +char buf[PATH_MAX ];/* get file for config (fd is always 3) */ snprintf (path,sizeof (path),"/proc/self/fd/%d" ,3 );
I wonder if we ever learn what stood behind this.
app/test/process.h get_current_prefix(char *prefix, int size) · line 227
Quoted 7 lines
app/test/process.h +2
-1@@ -216,10 +223,29 @@ get_current_prefix(char *prefix, int size)if (readlink (path, buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config"
JFYI it's /run/dpdk/my-prefix/config on my system, although example correctly demonstrates the algorithm.
app/test/process.h get_current_prefix(char *prefix, int size) · line 239
Quoted 12 lines
app/test/process.h +11
-0+* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ];
Can current prefix change? Can we verify that it is filled already and return immediately?
Quoted 13 lines
app/test/process.h +7
-1+if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif --
Quoted 1 line
2.51.0
Considering the refactor in the next commit, with or without suggested optimization,
Re: [PATCH v10 2/9] test: add new function to get current file-prefix arg
Quoted 8 lines · On Fri, 2 Jan 2026 12:50:14 +0000 Marat Khalili <[email protected]>…
app/test/process.h get_current_prefix(char *prefix, int size) · line 239
Quoted 6 lines
app/test/process.h +6
-0+/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ];Can current prefix change? Can we verify that it is filled already and return immediately?
file prefix comes from command line and does not change.
Given that it is used to setup memory and ipc it really can't change at runtime.
PATCH v10 3/9 test: refactor file prefix arg handling
In reply to Stephen HemmingerMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
--- app/test/process.h | 8 +-- app/test/test_eal_flags.c | 132 ++++++++++---------------------------- 2 files changed, 37 insertions(+), 103 deletions(-)
app/test/process.h +2 -6
@@ -213,14 +213,10 @@ file_prefix_arg(void)static inline char *get_current_prefix (char *prefix,int size) { -char path[64 ];char buf[PATH_MAX ]; -/* get file for config (fd is always 3) */ -snprintf (path,sizeof (path),"/proc/self/fd/%d" ,3 ); - -/* return NULL on error */ -if (readlink (path, buf,sizeof (buf)) ==-1 ) +/* get file for config (fd is always 3) return NULL on error */ +if (readlink ("/proc/self/fd/3" , buf,sizeof (buf)) ==-1 )return NULL ;/*
app/test/test_eal_flags.c +35 -97
233 lines not on this page · raw message
Signature · 1 line
2.51.0
RE: [PATCH v10 3/9] test: refactor file prefix arg handling
Please see a couple of nits inline.
Quoted 11 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>
Subject: [PATCH v10 3/9] test: refactor file prefix arg handlingMake setting up --file-prefix arg logic into a function, and avoid impossible case of file prefix path causing overflow of string.
Quoted 6 lines
Signed-off-by Stephen Hemminger <[email protected]>--- app/test/process.h | 8 +-- app/test/test_eal_flags.c | 132 ++++++++++---------------------------- 2 files changed, 37 insertions(+), 103 deletions(-)
// snip
app/test/test_eal_flags.c test_misc_flags(void) · line 125
Quoted 10 lines
app/test/test_eal_flags.c +2
-0@@ -121,6 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" +#define file_prefix "--file-prefix" +
Are we using it anywhere?
// snip
Quoted 40 lines
app/test/test_eal_flags.c +12
-13@@ -853,6 +802,18 @@ test_no_huge_flag(void)const char *argv2[] = {prgname, prefix, no_huge,"-m" ,DEFAULT_MEM_SIZE }; +if (launch_proc (argv1) !=0 ) { +printf ("Error - process did not run ok with --no-huge flag\n" ); +return -1 ; + } +if (launch_proc (argv2) !=0 ) { +printf ("Error - process did not run ok with --no-huge and -m flags\n" ); +return -1 ; + } +#ifdef RTE_EXEC_ENV_FREEBSD +/* no other tests are applicable to FreeBSD */ +return 0 ; +#else /* With --no-huge and --socket-mem */ const char *argv3[] = {prgname, prefix, no_huge,"--socket-mem=" DEFAULT_MEM_SIZE };@@ -867,19 +828,6 @@ test_no_huge_flag(void)const char *const argv6[] = {prgname, prefix, no_huge,"--huge-worker-stack=512" }; -if (launch_proc (argv1) !=0 ) { -printf ("Error - process did not run ok with --no-huge flag\n" ); -return -1 ; - } -if (launch_proc (argv2) !=0 ) { -printf ("Error - process did not run ok with --no-huge and -m flags\n" ); -return -1 ; - } -#ifdef RTE_EXEC_ENV_FREEBSD -/* no other tests are applicable to FreeBSD */ -return 0 ; -#endif -if (launch_proc (argv3) ==0 ) {printf ("Error - process run ok with --no-huge and --socket-mem " "flags\n" );
app/test/test_eal_flags.c test_no_huge_flag(void) · line 850
Quoted 5 lines
app/test/test_eal_flags.c +1
-0@@ -898,6 +846,7 @@ test_no_huge_flag(void)printf ("Error - process run ok with --no-huge and --huge-worker-stack=size flags" ); return -1; } +#endif return 0 ;
Duplicate return in FreeBSD case?
app/test/test_eal_flags.c test_no_huge_flag(void) · line 851
Quoted 1 line
app/test/test_eal_flags.c +0
-0}
// snip the rest
PATCH v10 4/9 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
Hugepages aren't used on Window or FreeBSD so this is skipped there.
--- app/test/test_eal_flags.c | 136 ++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 78 deletions(-)
app/test/test_eal_flags.c +58 -78
@@ -123,12 +123,17 @@ test_misc_flags(void)#define vdev "--vdev"#define file_prefix "--file-prefix" +#define FS_HUGETLB "hugetlbfs" +#define memtest "memtest"#define memtest1 "memtest1"#define memtest2 "memtest2"#define SOCKET_MEM_STRLEN (RTE_MAX_NUMA_NODES * 20)#define launch_proc (ARGV ) process_dup(ARGV, RTE_DIM(ARGV), __func__) +#ifdef RTE_EXEC_ENV_LINUX +#include <mntent.h> +enum hugepage_action {HUGEPAGE_CHECK_EXISTS =0 , HUGEPAGE_CHECK_LOCKED,
@@ -136,24 +141,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,45 +152,39 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error creating hugefile filename prefix\n" );return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error parsing /proc/mounts!\n" );return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); -printf ("Error reading %s: %s\n" , hugedir,strerror (errno)); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts); +printf ("Error reading %s: %s\n" , entry->mnt_dir,strerror (errno));return -1 ; }
@@ -222,10 +203,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -240,6 +221,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -253,10 +236,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -278,11 +258,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; } -#ifdef RTE_EXEC_ENV_LINUX /* * count the number of "node*" files in /sys/devices/system/node/ */
@@ -853,10 +832,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -864,9 +843,7 @@ test_misc_flags(void)#else const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL )return -1 ;
@@ -876,29 +853,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error opening /proc/mounts!\n" );return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
@@ -1167,6 +1139,17 @@ test_misc_flags(void)return -1 ; } +#ifdef RTE_EXEC_ENV_FREEBSD + +static int +test_file_prefix (void ) +{ +printf ("file_prefix not supported on FreeBSD, skipping test\n" ); +return TEST_SKIPPED ; +} + +#else +static int test_file_prefix (void ) {
@@ -1185,16 +1168,12 @@ test_file_prefix(void)* run in legacy mode, and not present at all after run in default * mem mode */ -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX ]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );return -1 ; } -#endif /* this should fail unless the test itself is run with "memtest" prefix */ const char *argv0[] = {prgname, mp_flag,"-m" ,
@@ -1430,6 +1409,7 @@ test_file_prefix(void) return 0; } +#endif /* This function writes in passed buf pointer a valid --socket-mem= option * for num_sockets then concatenates the provided suffix string.
Signature · 1 line
2.51.0
RE: [PATCH v10 4/9] test: avoid overflowing huge directory path
Quoted 10 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>
Subject: [PATCH v10 4/9] test: avoid overflowing huge directory pathModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
Quoted 8 lines
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
Hugepages aren't used on Window or FreeBSD so this is skipped there.
Signed-off-by Stephen Hemminger <[email protected]>
PATCH v10 5/9 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
app/test/test_table_acl.c +2 -2
@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */
@@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int
Signature · 1 line
2.51.0
RE: [PATCH v10 5/9] test: fix format overflow warning in ACL test
Quoted 46 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>; [email protected]; Anatoly Burakov <[email protected]>; Cristian Dumitrescu <[email protected]>; Pablo de Lara <[email protected]>
Subject: [PATCH v10 5/9] test: fix format overflow warning in ACL testThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
Fixes 5205954791cb ("app/test: packet framework unit tests")Signed-off-by Stephen Hemminger <[email protected]>Acked-by Anatoly Burakov <[email protected]>--- app/test/test_table_acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)app/test/test_table_acl.c +2
-3@@ -317,7 +317,7 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)* separated by ' : ', just ':'. It's a lot more readable and * cleaner, IMO. */ -char lines[][128 ] = { +static const char *const lines[] = {"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ "@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ "@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */ @@ -325,7 +325,7 @@ char lines[][128] = { "@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0", /* dst port check */ }; -char line[128 ]; +static char line[128 ]; static int --
Quoted 1 line
2.51.0
I feel like both could also be made local to setup_acl_pipeline.
With or without this optimization,
PATCH v10 6/9 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
RE: [PATCH v10 6/9] test: fix impossible format-truncation in cfgfiles
Quoted 45 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>; [email protected]; Anatoly Burakov <[email protected]>; Cristian Dumitrescu <[email protected]>; Bruce Richardson <[email protected]>
Subject: [PATCH v10 6/9] test: fix impossible format-truncation in cfgfilesAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible
snprintf()truncation.Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
Fixes be22019a58c4 ("test: restore cfgfile tests")Signed-off-by Stephen Hemminger <[email protected]>Acked-by Anatoly Burakov <[email protected]>--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)app/test/test_cfgfile.c +2
-7@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename); --
Quoted 2 lines
2.51.0
PATCH v10 7/9 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v10 8/9 test: fix overflow warnings in common code
In reply to Stephen HemmingerSeveral tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by allowing for bigger string.
--- app/test/test_mp_secondary.c | 14 ++++---------- app/test/test_pdump.c | 13 ++++--------- app/test/test_timer_secondary.c | 12 ++++-------- 3 files changed, 12 insertions(+), 27 deletions(-)
app/test/test_mp_secondary.c +4 -10
@@ -67,17 +67,11 @@ run_secondary_instances(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); - -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *argv1[] = {
app/test/test_pdump.c +4 -9
@@ -174,16 +174,11 @@ run_pdump_server_tests(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *const argv1[] = {
app/test/test_timer_secondary.c +4 -8
@@ -51,16 +51,12 @@ static int timer_secondary_spawn_wait(unsigned int lcore) {char coremask[10 ]; -#ifdef RTE_EXEC_ENV_LINUXAPP -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +const char *prefix; -get_current_prefix (tmp,sizeof (tmp)); + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ; -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif char const *argv[] = { prgname,"-c" , coremask,
Signature · 1 line
2.51.0
RE: [PATCH v10 8/9] test: fix overflow warnings in common code
Quoted 96 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>; Anatoly Burakov <[email protected]>; Reshma Pattan <[email protected]>; Erik Gabriel Carrillo <[email protected]>
Subject: [PATCH v10 8/9] test: fix overflow warnings in common codeSeveral tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by allowing for bigger string.
Signed-off-by Stephen Hemminger <[email protected]>--- app/test/test_mp_secondary.c | 14 ++++---------- app/test/test_pdump.c | 13 ++++--------- app/test/test_timer_secondary.c | 12 ++++-------- 3 files changed, 12 insertions(+), 27 deletions(-)app/test/test_mp_secondary.c +4
-10@@ -67,17 +67,11 @@ run_secondary_instances(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); - -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *argv1[] = {app/test/test_pdump.c +4
-9@@ -174,16 +174,11 @@ run_pdump_server_tests(void) {int ret =0 ;char coremask[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *const argv1[] = {app/test/test_timer_secondary.c +4
-9@@ -51,16 +51,12 @@ static int timer_secondary_spawn_wait(unsigned int lcore) {char coremask[10 ]; -#ifdef RTE_EXEC_ENV_LINUXAPP - char tmp[PATH_MAX] = {0}; - char prefix[PATH_MAX] = {0}; +const char *prefix; - get_current_prefix(tmp, sizeof(tmp)); + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ; - snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp); -#else - const char *prefix = ""; -#endifchar const *argv[] = { prgname,"-c" , coremask, --
Quoted 2 lines
2.51.0
(commit message looks slightly out of sync)
PATCH v10 9/9 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
RE: [PATCH v10 9/9] test: re-enable format-truncation warnings
Quoted 28 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Monday 29 December 2025 21:59
To: [email protected]
Cc: Stephen Hemminger <[email protected]>; Bruce Richardson <[email protected]>
Subject: [PATCH v10 9/9] test: re-enable format-truncation warningsTests should be built with warnings.
Signed-off-by Stephen Hemminger <[email protected]>Acked-by Bruce Richardson <[email protected]>--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)app/test/meson.build +0
-2@@ -260,7 +260,6 @@ cflags += no_wvla_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags --
Quoted 1 line
2.51.0
v11 cover tests: enable format-overflow and fix warnings
In reply to Stephen HemmingerThis series fixes compiler warnings in unit tests about format truncation. It also refactor some the related tests to be cleaner and use standard API's.
v11 - rebase to main
v10 - fix on FreeBSD
- rebase for 26.03
Stephen Hemminger (6):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
app/test/process.h | 44 +++++-- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 +++- app/test/test_eal_flags.c | 264 +++++++++++++------------------------- app/test/test_memzone.c | 2 +- app/test/test_table_acl.c | 30 ++--- 6 files changed, 157 insertions(+), 213 deletions(-)
Signature · 1 line
2.51.0
PATCH v11 1/6 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v11 2/6 test: add new function to get current file-prefix arg
In reply to Stephen HemmingerRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is NAME_MAX (128) not PATH_MAX(4096) so save a significant amount of space.
--- app/test/process.h | 44 +++++++++---- app/test/test_eal_flags.c | 132 ++++++++++---------------------------- 2 files changed, 68 insertions(+), 108 deletions(-)
app/test/process.h +33 -11
@@ -225,27 +225,49 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no argument passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg (void ) +{ +/* BSD target doesn't support prefixes at this point */ +return "" ; +} +#else +#ifdef RTE_EXEC_ENV_LINUX static inline char *get_current_prefix (char *prefix,int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; +char buf[PATH_MAX ]; -/* get file for config (fd is always 3) */ -snprintf (path,sizeof (path),"/proc/self/fd/%d" ,3 ); - -/* return NULL on error */ -if (readlink (path, buf,sizeof (buf)) ==-1 ) +/* get file for config (fd is always 3) return NULL on error */ +if (readlink ("/proc/self/fd/3" , buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config" +* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif
app/test/test_eal_flags.c +35 -97
233 lines not on this page · raw message
Signature · 1 line
2.51.0
PATCH v11 3/6 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
Hugepages aren't used on Window or FreeBSD so this is skipped there.
--- app/test/test_eal_flags.c | 142 ++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 83 deletions(-)
app/test/test_eal_flags.c +59 -83
@@ -121,7 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" -#define file_prefix "--file-prefix" + +#define FS_HUGETLB "hugetlbfs"#define memtest "memtest"#define memtest1 "memtest1"
@@ -129,6 +130,9 @@ test_misc_flags(void)#define SOCKET_MEM_STRLEN (RTE_MAX_NUMA_NODES * 20)#define launch_proc (ARGV ) process_dup(ARGV, RTE_DIM(ARGV), __func__) +#ifdef RTE_EXEC_ENV_LINUX +#include <mntent.h> +enum hugepage_action {HUGEPAGE_CHECK_EXISTS =0 , HUGEPAGE_CHECK_LOCKED,
@@ -136,24 +140,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,45 +151,39 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error creating hugefile filename prefix\n" );return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error parsing /proc/mounts!\n" );return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); -printf ("Error reading %s: %s\n" , hugedir,strerror (errno)); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts); +printf ("Error reading %s: %s\n" , entry->mnt_dir,strerror (errno));return -1 ; }
@@ -222,10 +202,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -240,6 +220,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -253,10 +235,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -278,11 +257,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; } -#ifdef RTE_EXEC_ENV_LINUX /* * count the number of "node*" files in /sys/devices/system/node/ */
@@ -810,10 +788,7 @@ test_no_huge_flag(void)printf ("Error - process did not run ok with --no-huge and -m flags\n" );return -1 ; } -#ifdef RTE_EXEC_ENV_FREEBSD -/* no other tests are applicable to FreeBSD */ -return 0 ; -#else +#ifndef RTE_EXEC_ENV_FREEBSD /* With --no-huge and --socket-mem */ const char *argv3[] = {prgname, prefix, no_huge,"--socket-mem=" DEFAULT_MEM_SIZE };
@@ -853,10 +828,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -864,9 +839,7 @@ test_misc_flags(void)#else const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL )return -1 ;
@@ -876,29 +849,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error opening /proc/mounts!\n" );return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
@@ -1167,6 +1135,17 @@ test_misc_flags(void)return -1 ; } +#ifdef RTE_EXEC_ENV_FREEBSD + +static int +test_file_prefix (void ) +{ +printf ("file_prefix not supported on FreeBSD, skipping test\n" ); +return TEST_SKIPPED ; +} + +#else +static int test_file_prefix (void ) {
@@ -1185,16 +1164,12 @@ test_file_prefix(void)* run in legacy mode, and not present at all after run in default * mem mode */ -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX ]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error - unable to get current prefix!\n" );return -1 ; } -#endif /* this should fail unless the test itself is run with "memtest" prefix */ const char *argv0[] = {prgname, mp_flag,"-m" ,
@@ -1430,6 +1405,7 @@ test_file_prefix(void) return 0; } +#endif /* This function writes in passed buf pointer a valid --socket-mem= option * for num_sockets then concatenates the provided suffix string.
Signature · 1 line
2.51.0
PATCH v11 4/6 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
app/test/test_table_acl.c +14 -16
@@ -312,22 +312,6 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)return 0 ; } -/* -* The format for these rules DO NOT need the port ranges to be -* separated by ' : ', just ':'. It's a lot more readable and -* cleaner, IMO. -*/ -char lines[][128 ] = { -"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ -"@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ -"@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */ -"@0.0.0.0/0 0.0.0.0/0 105:105 0:65535 0/0" ,/* src port check */ -"@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0" ,/* dst port check */ -}; - -char line[128 ]; - -static int setup_acl_pipeline (void ) {
@@ -343,6 +327,20 @@ setup_acl_pipeline(void)parse_5tuple parser;char acl_name[64 ]; +/* +* The format for these rules DO NOT need the port ranges to be +* separated by ' : ', just ':'. It's a lot more readable and +* cleaner, IMO. +*/ +static const char *const lines[] = { +"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ +"@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ +"@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */ +"@0.0.0.0/0 0.0.0.0/0 105:105 0:65535 0/0" ,/* src port check */ +"@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0" ,/* dst port check */ + }; +char line[LINE_MAX ]; +/* Pipeline configuration */ p =rte_pipeline_create (&pipeline_params);if (p ==NULL ) {
Signature · 1 line
2.51.0
PATCH v11 5/6 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v11 6/6 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
v12 cover test: fix format-truncation warnings
In reply to Stephen HemmingerThis patchset addresses the -Wformat-truncation warnings throughout the DPDK test suite, allowing the warning to be re-enabled.
The changes include:
- Properly sizing buffers using named constants (RTE_MEMZONE_NAMESIZE, NAME_MAX)
- Adding a common file_prefix_arg() helper to reduce code duplication
- Using standard library functions (getmntent) for mount parsing
- Checking snprintf() return values for potential truncation
- Converting problematic 2D arrays to pointer arrays
After these fixes, the final patch removes -Wno-format-truncation from the test build flags, ensuring these issues are caught in the future.
Several patches are marked for stable backport where they fix real potential bugs rather than just theoretical compiler warnings.
v12 - rebase to handle new eal_flags printf's
Stephen Hemminger (8):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in common code
test: re-enable format-truncation warnings
app/test/meson.build | 1 - app/test/process.h | 44 ++++-- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 269 +++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 14 +- app/test/test_pdump.c | 13 +- app/test/test_table_acl.c | 30 ++-- app/test/test_timer_secondary.c | 12 +- 10 files changed, 174 insertions(+), 241 deletions(-)
Signature · 1 line
2.51.0
PATCH v12 1/8 test: increase size of memzone name
The tests were using a name buffer of 20 characters which could overflow if number of memory zones got large. The upper limit is defined as RTE_MEMZONE_NAMESIZE so use that.
In function ‘test_memzone_free’,
inlined from ‘test_memzone’ at ../app/test/test_memzone.c:1117:6:../app/test/test_memzone.c:52:35: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:52:35: note: directive argument in the range [0, 2147483647]
52 | #define TEST_MEMZONE_NAME(suffix) "MZ_TEST_" suffix
| ^~~~~~~~~~../app/test/test_memzone.c:931:46: note: in expansion of macro ‘TEST_MEMZONE_NAME’
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~
../app/test/test_memzone.c:931:17: note: ‘snprintf’ output between 18 and 27 bytes into a destination of size 20
931 | snprintf(name, sizeof(name), TEST_MEMZONE_NAME("tempzone%u"),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
932 | i);
|--- app/test/test_memzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
app/test/test_memzone.c +1 -1
@@ -872,7 +872,7 @@ test_memzone_free(void) {const struct rte_memzone **mz;int i; -char name[20 ]; +char name[RTE_MEMZONE_NAMESIZE ];int rc =-1 ; mz = rte_calloc("memzone_test", rte_memzone_max_get() + 1,
Signature · 1 line
2.51.0
PATCH v12 2/8 test: add new function to get current file-prefix arg
In reply to Stephen HemmingerRefactor so that there is one function to get current argument for "--file-prefix=XXX".
The code does not need to initialize all these 4K buffers since they are directly used by functions that set them. The maximum sizeof of the current DPDK prefix is NAME_MAX (128) not PATH_MAX(4096) so save a significant amount of space.
--- app/test/process.h | 44 +++++++++---- app/test/test_eal_flags.c | 133 +++++++++++--------------------------- 2 files changed, 71 insertions(+), 106 deletions(-)
app/test/process.h +33 -11
@@ -225,27 +225,49 @@ process_dup(const char *const argv[], int numargs, const char *env_value)return status; } -/* FreeBSD doesn't support file prefixes, so force compile failures for any -* tests attempting to use this function on FreeBSD. -*/ +/* FreeBSD doesn't support file prefixes, so no argument passed. */ +#ifdef RTE_EXEC_ENV_FREEBSD +static inline const char * +file_prefix_arg (void ) +{ +/* BSD target doesn't support prefixes at this point */ +return "" ; +} +#else +#ifdef RTE_EXEC_ENV_LINUX static inline char *get_current_prefix (char *prefix,int size) { -char path[PATH_MAX ] = {0 }; -char buf[PATH_MAX ] = {0 }; +char buf[PATH_MAX ]; -/* get file for config (fd is always 3) */ -snprintf (path,sizeof (path),"/proc/self/fd/%d" ,3 ); - -/* return NULL on error */ -if (readlink (path, buf,sizeof (buf)) ==-1 ) +/* get file for config (fd is always 3) return NULL on error */ +if (readlink ("/proc/self/fd/3" , buf,sizeof (buf)) ==-1 )return NULL ; -/* get the prefix */ +/* +* path should be something like "/var/run/dpdk/config" +* which results in prefix of "dpdk" +*/ rte_basename (dirname (buf), prefix, size); +return prefix; +} + +/* Return a --file-prefix=XXXX argument or NULL */ +static inline const char * +file_prefix_arg (void ) +{ +static char prefix[NAME_MAX +sizeof ("--file-prefix=" )]; +char tmp[NAME_MAX ]; +if (get_current_prefix (tmp,sizeof (tmp)) ==NULL ) { +fprintf (stderr,"Error - unable to get current prefix!\n" ); +return NULL ; + } + +snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp);return prefix; +#endif }#endif
app/test/test_eal_flags.c +38 -95
258 lines not on this page · raw message
Signature · 1 line
2.51.0
PATCH v12 3/8 test: avoid overflowing huge directory path
In reply to Stephen HemmingerModify the part of the test that is scanning for hugepages mount point to use existing getmntent library calls.
As a result, the maximum mount path is smaller which is ok since this is all handled by the standard library. And the resulting huge path arguments don't overflow.
Hugepages aren't used on Window or FreeBSD so this is skipped there.
--- app/test/test_eal_flags.c | 146 ++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 85 deletions(-)
app/test/test_eal_flags.c +61 -85
@@ -121,7 +121,8 @@ test_misc_flags(void)#define no_shconf "--no-shconf"#define allow "--allow"#define vdev "--vdev" -#define file_prefix "--file-prefix" + +#define FS_HUGETLB "hugetlbfs"#define memtest "memtest"#define memtest1 "memtest1"
@@ -129,6 +130,9 @@ test_misc_flags(void)#define SOCKET_MEM_STRLEN (RTE_MAX_NUMA_NODES * 20)#define launch_proc (ARGV ) process_dup(ARGV, RTE_DIM(ARGV), __func__) +#ifdef RTE_EXEC_ENV_LINUX +#include <mntent.h> +enum hugepage_action {HUGEPAGE_CHECK_EXISTS =0 , HUGEPAGE_CHECK_LOCKED,
@@ -136,24 +140,6 @@ enum hugepage_action { HUGEPAGE_INVALID }; -/* if string contains a hugepage path */ -static int -get_hugepage_path (char * src,int src_len,char * dst,int dst_len) -{ -#define NUM_TOKENS 4 -char *tokens[NUM_TOKENS ]; - -/* if we couldn't properly split the string */ -if (rte_strsplit (src, src_len, tokens,NUM_TOKENS ,' ' ) <NUM_TOKENS ) -return 0 ; - -if (strncmp (tokens[2 ],"hugetlbfs" ,sizeof ("hugetlbfs" )) ==0 ) { -strlcpy (dst, tokens[1 ], dst_len); -return 1 ; - } -return 0 ; -} -/* * Cycles through hugepage directories and looks for hugepage * files associated with a given prefix. Depending on value of
@@ -165,46 +151,40 @@ get_hugepage_path(char * src, int src_len, char * dst, int dst_len)* Returns -1 if it encounters an error */ static int -process_hugefiles (const char * prefix,enum hugepage_action action) +process_hugefiles (const char *prefix,enum hugepage_action action) { -FILE * hugedir_handle =NULL ; -DIR * hugepage_dir =NULL ; -struct dirent *dirent =NULL ; - -char hugefile_prefix[PATH_MAX ] = {0 }; -char hugedir[PATH_MAX ] = {0 }; -char line[PATH_MAX ] = {0 }; - -int fd, lck_result, result =0 ; +const struct mntent *entry; +char hugefile_prefix[PATH_MAX ]; +int result =0 ; -const int prefix_len =snprintf (hugefile_prefix, -sizeof (hugefile_prefix),"%smap_" , prefix); -if (prefix_len <=0 || prefix_len >= (int )sizeof (hugefile_prefix) - || prefix_len >= (int )sizeof (dirent->d_name)) { +const int prefix_len =snprintf (hugefile_prefix,sizeof (hugefile_prefix),"%smap_" , prefix); +if (prefix_len <=0 || prefix_len >=NAME_MAX ) {printf ("Error (line %d) - cannot create hugefile filename prefix\n" , __LINE__);return -1 ; }/* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error (line %d) - cannot parse /proc/mounts!\n" , __LINE__);return -1 ; } -/* read and parse script output */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { +/* foreach mountpoint */ +while ((entry =getmntent (mounts)) !=NULL ) { +DIR *hugepage_dir; +struct dirent *dirent; -/* check if we have a hugepage filesystem path */ -if (!get_hugepage_path (line,sizeof (line), hugedir,sizeof (hugedir))) +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) !=0 )continue ;/* check if directory exists */ -if ((hugepage_dir =opendir (hugedir)) ==NULL ) { -fclose (hugedir_handle); + hugepage_dir =opendir (entry->mnt_dir); +if (hugepage_dir ==NULL ) { +endmntent (mounts);printf ("Error (line %d) - cannot open %s: %s\n" , - __LINE__, hugedir,strerror (errno)); + __LINE__, entry->mnt_dir,strerror (errno));return -1 ; }
@@ -217,7 +197,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) {/* file exists, return */ printf ("Hugepage file %s/%s exists, matching prefix %s\n" , - hugedir, dirent->d_name, hugefile_prefix); + entry->mnt_dir, dirent->d_name, hugefile_prefix);closedir (hugepage_dir); result =1 ; goto end;
@@ -225,10 +205,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_DELETE : { -char file_path[PATH_MAX ] = {0 }; +char file_path[PATH_MAX ];snprintf (file_path,sizeof (file_path), -"%s/%s" , hugedir, dirent->d_name); +"%s/%s" , entry->mnt_dir, dirent->d_name);/* remove file */ if (remove (file_path) <0 ) {
@@ -244,6 +224,8 @@ process_hugefiles(const char * prefix, enum hugepage_action action)break ;case HUGEPAGE_CHECK_LOCKED : { +int fd; +/* try and lock the file */ fd =openat (dirfd (hugepage_dir), dirent->d_name,O_RDONLY );
@@ -257,10 +239,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) }/* non-blocking lock */ - lck_result =flock (fd,LOCK_EX |LOCK_NB ); - -/* if lock succeeds, there's something wrong */ -if (lck_result !=-1 ) { +if (flock (fd,LOCK_EX |LOCK_NB ) !=-1 ) { result =0 ;/* unlock the resulting lock */
@@ -271,7 +250,7 @@ process_hugefiles(const char * prefix, enum hugepage_action action) } result =1 ;printf ("Hugepage file %s/%s is locked\n" , - hugedir, dirent->d_name); + entry->mnt_dir, dirent->d_name);close (fd); }break ;
@@ -284,11 +263,10 @@ process_hugefiles(const char * prefix, enum hugepage_action action)closedir (hugepage_dir); }/* read /proc/mounts */ end: -fclose (hugedir_handle); +endmntent (mounts);return result; } -#ifdef RTE_EXEC_ENV_LINUX /* * count the number of "node*" files in /sys/devices/system/node/ */
@@ -828,10 +806,7 @@ test_no_huge_flag(void)printf ("Error - process did not run ok with --no-huge and -m flags\n" );return -1 ; } -#ifdef RTE_EXEC_ENV_FREEBSD -/* no other tests are applicable to FreeBSD */ -return 0 ; -#else +#ifndef RTE_EXEC_ENV_FREEBSD /* With --no-huge and --socket-mem */ const char *argv3[] = {prgname, prefix, no_huge,"--socket-mem=" DEFAULT_MEM_SIZE };
@@ -873,10 +848,10 @@ test_no_huge_flag(void)static int test_misc_flags (void ) { -char hugepath[PATH_MAX ] = {0 }; -char hugepath_dir[PATH_MAX ] = {0 }; -char hugepath_dir2[PATH_MAX ] = {0 }; -char hugepath_dir3[PATH_MAX ] = {0 }; +const char *hugepath ="" ; +char hugepath_dir[PATH_MAX ]; +char hugepath_dir2[PATH_MAX ]; +char hugepath_dir3[PATH_MAX ];#ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix ="" ;
@@ -884,9 +859,7 @@ test_misc_flags(void)#else const char *prefix =file_prefix_arg ();const char * nosh_prefix ="--file-prefix=noshconf" ; -FILE * hugedir_handle =NULL ; -char line[PATH_MAX ] = {0 }; -unsigned i, isempty =1 ; +struct mntent *entry;if (prefix ==NULL ) {printf ("Error (line %d) - unable to get current prefix!\n" , __LINE__);
@@ -898,29 +871,24 @@ test_misc_flags(void)*/ /* get hugetlbfs mountpoints from /proc/mounts */ - hugedir_handle =fopen ("/proc/mounts" ,"r" ); - -if (hugedir_handle ==NULL ) { +FILE *mounts =setmntent ("/proc/mounts" ,"r" ); +if (mounts ==NULL ) {printf ("Error (line %d) - cannot open /proc/mounts!\n" , __LINE__);return -1 ; } -/* read /proc/mounts */ -while (fgets (line,sizeof (line), hugedir_handle) !=NULL ) { - -/* find first valid hugepath */ -if (get_hugepage_path (line,sizeof (line), hugepath,sizeof (hugepath))) +/* foreach mount point */ + hugepath =NULL ; +while ((entry =getmntent (mounts)) !=NULL ) { +/* only want hugetlbfs filesystems */ +if (strcmp (entry->mnt_type,FS_HUGETLB ) ==0 ) { + hugepath =strdupa (entry->mnt_dir);break ; + } } +endmntent (mounts); -fclose (hugedir_handle); - -/* check if path is not empty */ -for (i =0 ; i <sizeof (hugepath); i++) -if (hugepath[i] !='\0' ) - isempty =0 ; - -if (isempty) { +if (hugepath ==NULL ) {printf ("No mounted hugepage dir found!\n" );return -1 ; }
@@ -1204,6 +1172,17 @@ test_misc_flags(void)return -1 ; } +#ifdef RTE_EXEC_ENV_FREEBSD + +static int +test_file_prefix (void ) +{ +printf ("file_prefix not supported on FreeBSD, skipping test\n" ); +return TEST_SKIPPED ; +} + +#else +static int test_file_prefix (void ) {
@@ -1222,16 +1201,12 @@ test_file_prefix(void)* run in legacy mode, and not present at all after run in default * mem mode */ -char prefix[PATH_MAX ] ="" ; +char prefix[PATH_MAX ]; -#ifdef RTE_EXEC_ENV_FREEBSD -return 0 ; -#else if (get_current_prefix (prefix,sizeof (prefix)) ==NULL ) {printf ("Error (line %d) - unable to get current prefix!\n" , __LINE__);return -1 ; } -#endif /* this should fail unless the test itself is run with "memtest" prefix */ const char *argv0[] = {prgname, mp_flag,"-m" ,
@@ -1476,6 +1451,7 @@ test_file_prefix(void) return 0; } +#endif /* This function writes in passed buf pointer a valid --socket-mem= option * for num_sockets then concatenates the provided suffix string.
Signature · 1 line
2.51.0
PATCH v12 4/8 test: fix format overflow warning in ACL test
In reply to Stephen HemmingerThis test has an array of input lines, but the two dimensional array confuses compiler length checks. Convert to an array of pointers to fixed strings which avoids the problem. Make both variables static since not shared with other code.
--- app/test/test_table_acl.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
app/test/test_table_acl.c +14 -16
@@ -312,22 +312,6 @@ parse_cb_ipv4_rule_del(char *str, struct rte_table_acl_rule_delete_params *v)return 0 ; } -/* -* The format for these rules DO NOT need the port ranges to be -* separated by ' : ', just ':'. It's a lot more readable and -* cleaner, IMO. -*/ -char lines[][128 ] = { -"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ -"@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ -"@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */ -"@0.0.0.0/0 0.0.0.0/0 105:105 0:65535 0/0" ,/* src port check */ -"@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0" ,/* dst port check */ -}; - -char line[128 ]; - -static int setup_acl_pipeline (void ) {
@@ -343,6 +327,20 @@ setup_acl_pipeline(void)parse_5tuple parser;char acl_name[64 ]; +/* +* The format for these rules DO NOT need the port ranges to be +* separated by ' : ', just ':'. It's a lot more readable and +* cleaner, IMO. +*/ +static const char *const lines[] = { +"@0.0.0.0/0 0.0.0.0/0 0:65535 0:65535 2/0xff" ,/* Protocol check */ +"@192.168.3.1/32 0.0.0.0/0 0:65535 0:65535 0/0" ,/* Src IP checl */ +"@0.0.0.0/0 10.4.4.1/32 0:65535 0:65535 0/0" ,/* dst IP check */ +"@0.0.0.0/0 0.0.0.0/0 105:105 0:65535 0/0" ,/* src port check */ +"@0.0.0.0/0 0.0.0.0/0 0:65535 206:206 0/0" ,/* dst port check */ + }; +char line[LINE_MAX ]; +/* Pipeline configuration */ p =rte_pipeline_create (&pipeline_params);if (p ==NULL ) {
Signature · 1 line
2.51.0
PATCH v12 5/8 test: fix impossible format-truncation in cfgfiles
In reply to Stephen HemmingerAlthough, it is not possible on Linux (which always uses /tmp) the compiler complains about possible snprintf() truncation.
Use existing code to make empty tmp file which puts the OS specific code in one spot and avoids any races if two tests are run at once.
--- app/test/test_cfgfile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
app/test/test_cfgfile.c +2 -6
@@ -178,12 +178,8 @@ test_cfgfile_realloc_sections(void) ret =remove (filename);TEST_ASSERT_SUCCESS (ret,"Failed to remove file" ); -char tmp[PATH_MAX ] ="/tmp/" ; -#ifdef RTE_EXEC_ENV_WINDOWS - ret =GetTempPathA (sizeof (tmp), tmp); -TEST_ASSERT (ret >0 ,"Failed to get tmp directory" ); -#endif -snprintf (filename,sizeof (filename),"%s%s" , tmp,"cfg_save.ini" ); + ret =make_tmp_file (filename,"save" ,"" ); +TEST_ASSERT (ret ==0 ,"Failed to make empty tmp filename for save" ); ret =rte_cfgfile_save (cfgfile, filename);TEST_ASSERT_SUCCESS (ret,"Failed to save to %s" , filename);
Signature · 1 line
2.51.0
PATCH v12 6/8 test: fix format overflow in cryptodev test
In reply to Stephen HemmingerThe vdev args buffer could in theory get truncated if there were lots of sockets or lcores. Handle that by checking return value of snprintf.
--- app/test/test_cryptodev.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
app/test/test_cryptodev.c +18 -4
@@ -17713,8 +17713,16 @@ scheduler_testsuite_setup(void)RTE_LCORE_FOREACH_WORKER (i) {if (worker_core_count >1 )break ; -snprintf (vdev_args,sizeof (vdev_args), -"%s%d" , temp_str, i); + ret =snprintf (vdev_args,sizeof (vdev_args), +"%s%d" , temp_str, i); + +/* If too many args the result will have been truncated */ +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +strcpy (temp_str, vdev_args);strlcat (temp_str,";" ,sizeof (temp_str)); worker_core_count++;
@@ -17728,8 +17736,14 @@ scheduler_testsuite_setup(void)return TEST_FAILED ; }strcpy (temp_str, vdev_args); -snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , - temp_str, socket_id); + ret =snprintf (vdev_args,sizeof (vdev_args),"%s,socket_id=%d" , + temp_str, socket_id); +if (ret >=VDEV_ARGS_SIZE ) { +RTE_LOG (ERR ,USER1 , +"Cryptodev scheduler test vdev arg size exceeded\n" ); +return TEST_FAILED ; + } +RTE_LOG (DEBUG ,USER1 ,"vdev_args: %s\n" , vdev_args); nb_devs = rte_cryptodev_device_count_by_driver( rte_cryptodev_driver_id_get(
Signature · 1 line
2.51.0
PATCH v12 7/8 test: fix overflow warnings in common code
In reply to Stephen HemmingerSeveral tests lookup the current file prefix and format up a filename argument. Avoid any possible format overflows by using new file prefix function.
--- app/test/test_mp_secondary.c | 14 ++++---------- app/test/test_pdump.c | 13 ++++--------- app/test/test_timer_secondary.c | 12 ++++-------- 3 files changed, 12 insertions(+), 27 deletions(-)
app/test/test_mp_secondary.c +4 -10
@@ -67,17 +67,11 @@ run_secondary_instances(void) {int ret =0 ;char core_str[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); - -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *argv1[] = {
app/test/test_pdump.c +4 -9
@@ -174,16 +174,11 @@ run_pdump_server_tests(void) {int ret =0 ;char core_str[10 ]; +const char *prefix; -#ifdef RTE_EXEC_ENV_LINUX -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; - -get_current_prefix (tmp,sizeof (tmp)); -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ;/* good case, using secondary */ const char *const argv1[] = {
app/test/test_timer_secondary.c +4 -8
@@ -51,16 +51,12 @@ static int timer_secondary_spawn_wait(unsigned int lcore) {char core_str[10 ]; -#ifdef RTE_EXEC_ENV_LINUXAPP -char tmp[PATH_MAX ] = {0 }; -char prefix[PATH_MAX ] = {0 }; +const char *prefix; -get_current_prefix (tmp,sizeof (tmp)); + prefix =file_prefix_arg (); +if (prefix ==NULL ) +return -1 ; -snprintf (prefix,sizeof (prefix),"--file-prefix=%s" , tmp); -#else -const char *prefix ="" ; -#endif char const *argv[] = { prgname,"-l" , core_str,
Signature · 1 line
2.51.0
PATCH v12 8/8 test: re-enable format-truncation warnings
In reply to Stephen HemmingerTests should be built with warnings.
--- app/test/meson.build | 1 - 1 file changed, 1 deletion(-)
app/test/meson.build +0 -1
@@ -261,7 +261,6 @@ cflags += no_shadow_cflag extra_flags = [ # Strict-aliasing rules are violated by uint8_t[] to context size casts. '-fno-strict-aliasing', - '-Wno-format-truncation', ] foreach arg: extra_flags
Signature · 1 line
2.51.0
Re: [PATCH v12 0/8] test: fix format-truncation warnings
In reply to Stephen HemmingerQuoted 29 lines · Stephen Hemminger
This patchset addresses the -Wformat-truncation warnings throughout the DPDK test suite, allowing the warning to be re-enabled.
The changes include:
- Properly sizing buffers using named constants (RTE_MEMZONE_NAMESIZE,NAME_MAX)
- Adding a commonfile_prefix_arg()helper to reduce code duplication
- Using standard library functions (getmntent) for mount parsing
- Checkingsnprintf()return values for potential truncation
- Converting problematic 2D arrays to pointer arraysAfter these fixes, the final patch removes -Wno-format-truncation from the test build flags, ensuring these issues are caught in the future.
Several patches are marked for stable backport where they fix real potential bugs rather than just theoretical compiler warnings.
v12 - rebase to handle new
eal_flagsprintf'sStephen Hemminger (8):
test: increase size of memzone name
test: add new function to get current file-prefix arg
test: avoid overflowing huge directory path
test: fix format overflow warning in ACL test
test: fix impossible format-truncation in cfgfiles
test: fix format overflow in cryptodev test
test: fix overflow warnings in common code
test: re-enable format-truncation warnings
Quoted 11 lines
app/test/meson.build | 1 - app/test/process.h | 44 ++++-- app/test/test_cfgfile.c | 8 +- app/test/test_cryptodev.c | 22 ++- app/test/test_eal_flags.c | 269 +++++++++++--------------------- app/test/test_memzone.c | 2 +- app/test/test_mp_secondary.c | 14 +- app/test/test_pdump.c | 13 +- app/test/test_table_acl.c | 30 ++-- app/test/test_timer_secondary.c | 12 +- 10 files changed, 174 insertions(+), 241 deletions(-)
Series applied, thanks.
Signature · 1 line
David Marchand