diff mbox series

[v6,05/10] test-pmd: avoid undefined behavior

Message ID 1740414265-12217-6-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State Rejected
Delegated to: David Marchand
Headers
Series enable "app" to be compiled with MSVC |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andre Muezerie Feb. 24, 2025, 4:24 p.m. UTC 
Compiling with MSVC results in warnings like below:

app/test-pmd/cmdline.c(9023): warning C5101: use of preprocessor
    directive in function-like macro argument list is undefined behavior

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test-pmd/cmdline.c | 12 ++++++++----
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 10 insertions(+), 4 deletions(-)

Comments

David Marchand April 10, 2025, 4:05 p.m. UTC | #1 
On Mon, Feb 24, 2025 at 5:25 PM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> Compiling with MSVC results in warnings like below:
>
> app/test-pmd/cmdline.c(9023): warning C5101: use of preprocessor
>     directive in function-like macro argument list is undefined behavior
>
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  app/test-pmd/cmdline.c | 12 ++++++++----
>  app/test-pmd/testpmd.h |  2 ++
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 2afcf916c0..d0b47eac8f 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -8980,6 +8980,14 @@ dump_socket_mem(FILE *f)
>         last_total = total;
>  }
>
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +int
> +rte_trace_save(void)
> +{
> +       return TEST_SKIPPED;
> +}
> +#endif
> +

- I don't like this fake declaration.
It does not comply with this function documented API.
And there are other stubs for the trace API in eal/windows, where this
could be a better place.

- TEST_SKIPPED 77 is some known error code when invoking unit test via
meson, which does not make sense with testpmd.


I sent a simpler fix, could you have a look?
https://patchwork.dpdk.org/project/dpdk/patch/20250410160237.3067629-1-david.marchand@redhat.com/
Andre Muezerie April 11, 2025, 12:06 a.m. UTC | #2 
On Thu, Apr 10, 2025 at 06:05:10PM +0200, David Marchand wrote:
> On Mon, Feb 24, 2025 at 5:25 PM Andre Muezerie
> <andremue@linux.microsoft.com> wrote:
> >
> > Compiling with MSVC results in warnings like below:
> >
> > app/test-pmd/cmdline.c(9023): warning C5101: use of preprocessor
> >     directive in function-like macro argument list is undefined behavior
> >
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > ---
> >  app/test-pmd/cmdline.c | 12 ++++++++----
> >  app/test-pmd/testpmd.h |  2 ++
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index 2afcf916c0..d0b47eac8f 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -8980,6 +8980,14 @@ dump_socket_mem(FILE *f)
> >         last_total = total;
> >  }
> >
> > +#ifdef RTE_EXEC_ENV_WINDOWS
> > +int
> > +rte_trace_save(void)
> > +{
> > +       return TEST_SKIPPED;
> > +}
> > +#endif
> > +
> 
> - I don't like this fake declaration.
> It does not comply with this function documented API.
> And there are other stubs for the trace API in eal/windows, where this
> could be a better place.
> 
> - TEST_SKIPPED 77 is some known error code when invoking unit test via
> meson, which does not make sense with testpmd.
> 
> 
> I sent a simpler fix, could you have a look?
> https://patchwork.dpdk.org/project/dpdk/patch/20250410160237.3067629-1-david.marchand@redhat.com/
> 

I like the solution you proposed. Thanks!

> 
> -- 
> David Marchand
diff mbox series

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2afcf916c0..d0b47eac8f 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8980,6 +8980,14 @@  dump_socket_mem(FILE *f)
 	last_total = total;
 }
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+int
+rte_trace_save(void)
+{
+	return TEST_SKIPPED;
+}
+#endif
+
 static void cmd_dump_parsed(void *parsed_result,
 			    __rte_unused struct cmdline *cl,
 			    __rte_unused void *data)
@@ -9002,10 +9010,8 @@  static void cmd_dump_parsed(void *parsed_result,
 		rte_devargs_dump(stdout);
 	else if (!strcmp(res->dump, "dump_lcores"))
 		rte_lcore_dump(stdout);
-#ifndef RTE_EXEC_ENV_WINDOWS
 	else if (!strcmp(res->dump, "dump_trace"))
 		rte_trace_save();
-#endif
 	else if (!strcmp(res->dump, "dump_log_types"))
 		rte_log_dump(stdout);
 }
@@ -9020,9 +9026,7 @@  static cmdline_parse_token_string_t cmd_dump_dump =
 		"dump_mempool#"
 		"dump_devargs#"
 		"dump_lcores#"
-#ifndef RTE_EXEC_ENV_WINDOWS
 		"dump_trace#"
-#endif
 		"dump_log_types");
 
 static cmdline_parse_inst_t cmd_dump = {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a933fb433a..de8e8f8dca 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -84,6 +84,8 @@  extern volatile uint8_t f_quit;
 /* Maximum number of pools supported per Rx queue */
 #define MAX_MEMPOOL 8
 
+#define TEST_SKIPPED 77
+
 typedef uint32_t lcoreid_t;
 typedef uint16_t portid_t;
 typedef uint16_t queueid_t;