eal: add const to init function parameter

Message ID 20231102181148.56930-1-bruce.richardson@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series eal: add const to init function parameter |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation fail Compilation issues
ci/github-robot: build fail github build: failed
ci/iol-compile-arm64-testing fail Testing issues
ci/iol-unit-amd64-testing fail Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing fail Testing issues
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-sample-apps-testing warning Testing issues
ci/iol-unit-arm64-testing fail Testing issues

Commit Message

Bruce Richardson Nov. 2, 2023, 6:11 p.m. UTC
  Change the parameter type of argv parameter to rte_eal_init from
"char **" to "char * const *", since we don't modify the argv pointers
passed in.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/common/eal_common_options.c | 2 +-
 lib/eal/common/eal_options.h        | 2 +-
 lib/eal/freebsd/eal.c               | 4 ++--
 lib/eal/include/rte_eal.h           | 2 +-
 lib/eal/linux/eal.c                 | 4 ++--
 lib/eal/windows/eal.c               | 4 ++--
 6 files changed, 9 insertions(+), 9 deletions(-)
  

Comments

Stephen Hemminger Nov. 2, 2023, 6:55 p.m. UTC | #1
On Thu,  2 Nov 2023 18:11:48 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> From: Bruce Richardson <bruce.richardson@intel.com>
> To: dev@dpdk.org
> Cc: stephen@networkplumber.org,  Bruce Richardson <bruce.richardson@intel.com>
> Subject: [PATCH] eal: add const to init function parameter
> Date: Thu,  2 Nov 2023 18:11:48 +0000
> X-Mailer: git-send-email 2.39.2
> 
> Change the parameter type of argv parameter to rte_eal_init from
> "char **" to "char * const *", since we don't modify the argv pointers
> passed in.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Looks good but probably needs to cascade down a few more levels
  
Konstantin Ananyev Nov. 3, 2023, 1:06 a.m. UTC | #2
02.11.2023 18:11, Bruce Richardson пишет:
> Change the parameter type of argv parameter to rte_eal_init from
> "char **" to "char * const *", since we don't modify the argv pointers
> passed in.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   lib/eal/common/eal_common_options.c | 2 +-
>   lib/eal/common/eal_options.h        | 2 +-
>   lib/eal/freebsd/eal.c               | 4 ++--
>   lib/eal/include/rte_eal.h           | 2 +-
>   lib/eal/linux/eal.c                 | 4 ++--
>   lib/eal/windows/eal.c               | 4 ++--
>   6 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index a6d21f1cba..7927eb1f1d 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -206,7 +206,7 @@ handle_eal_info_request(const char *cmd, const char *params __rte_unused,
>   }
>   
>   int
> -eal_save_args(int argc, char **argv)
> +eal_save_args(int argc, char * const *argv)
>   {
>   	int i, j;
>   
> diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
> index 3cc9cb6412..21ab2492fc 100644
> --- a/lib/eal/common/eal_options.h
> +++ b/lib/eal/common/eal_options.h
> @@ -105,7 +105,7 @@ int eal_check_common_options(struct internal_config *internal_cfg);
>   void eal_common_usage(void);
>   enum rte_proc_type_t eal_proc_type_detect(void);
>   int eal_plugins_init(void);
> -int eal_save_args(int argc, char **argv);
> +int eal_save_args(int argc, char * const *argv);
>   int handle_eal_info_request(const char *cmd, const char *params __rte_unused,
>   		struct rte_tel_data *d);
>   
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index 568e06e9ed..f241e7dd5e 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -365,7 +365,7 @@ eal_get_hugepage_mem_size(void)
>   
>   /* Parse the arguments for --log-level only */
>   static void
> -eal_log_level_parse(int argc, char **argv)
> +eal_log_level_parse(int argc, char * const *argv)
>   {
>   	int opt;
>   	char **argvopt;
> @@ -577,7 +577,7 @@ static void rte_eal_init_alert(const char *msg)
>   
>   /* Launch threads, called at application init(). */
>   int
> -rte_eal_init(int argc, char **argv)
> +rte_eal_init(int argc, char * const *argv)
>   {
>   	int i, fctret, ret;
>   	static uint32_t run_once;
> diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
> index cd318ee141..e0a74865ad 100644
> --- a/lib/eal/include/rte_eal.h
> +++ b/lib/eal/include/rte_eal.h
> @@ -109,7 +109,7 @@ int rte_eal_iopl_init(void);
>    *
>    *     ENOEXEC indicates that a service core failed to launch successfully.
>    */
> -int rte_eal_init(int argc, char **argv);
> +int rte_eal_init(int argc, char * const *argv);
>   
>   /**
>    * Clean up the Environment Abstraction Layer (EAL)
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 57da058cec..a56b85f2ce 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -548,7 +548,7 @@ eal_parse_vfio_vf_token(const char *vf_token)
>   
>   /* Parse the arguments for --log-level only */
>   static void
> -eal_log_level_parse(int argc, char **argv)
> +eal_log_level_parse(int argc, char * const *argv)
>   {
>   	int opt;
>   	char **argvopt;
> @@ -964,7 +964,7 @@ eal_worker_thread_create(unsigned int lcore_id)
>   
>   /* Launch threads, called at application init(). */
>   int
> -rte_eal_init(int argc, char **argv)
> +rte_eal_init(int argc, char * const *argv)
>   {
>   	int i, fctret, ret;
>   	static RTE_ATOMIC(uint32_t) run_once;
> diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
> index 7ec2152211..1abb2eb2f7 100644
> --- a/lib/eal/windows/eal.c
> +++ b/lib/eal/windows/eal.c
> @@ -98,7 +98,7 @@ eal_usage(const char *prgname)
>   
>   /* Parse the arguments for --log-level only */
>   static void
> -eal_log_level_parse(int argc, char **argv)
> +eal_log_level_parse(int argc, char * const *argv)
>   {
>   	int opt;
>   	char **argvopt;
> @@ -273,7 +273,7 @@ rte_eal_cleanup(void)
>   
>   /* Launch threads, called at application init(). */
>   int
> -rte_eal_init(int argc, char **argv)
> +rte_eal_init(int argc, char * const *argv)
>   {
>   	int i, fctret, bscan;
>   	const struct rte_config *config = rte_eal_get_configuration();

Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
  
Bruce Richardson Nov. 3, 2023, 10:17 a.m. UTC | #3
On Thu, Nov 02, 2023 at 11:55:41AM -0700, Stephen Hemminger wrote:
> On Thu,  2 Nov 2023 18:11:48 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > To: dev@dpdk.org
> > Cc: stephen@networkplumber.org,  Bruce Richardson <bruce.richardson@intel.com>
> > Subject: [PATCH] eal: add const to init function parameter
> > Date: Thu,  2 Nov 2023 18:11:48 +0000
> > X-Mailer: git-send-email 2.39.2
> > 
> > Change the parameter type of argv parameter to rte_eal_init from
> > "char **" to "char * const *", since we don't modify the argv pointers
> > passed in.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Looks good but probably needs to cascade down a few more levels

Yep. I only did a quick compile test, which passed, but I see the CI has
thrown up a bunch more issues. I'll do a v2 later.

/Bruce
  
Bruce Richardson Nov. 3, 2023, 12:44 p.m. UTC | #4
On Fri, Nov 03, 2023 at 10:17:49AM +0000, Bruce Richardson wrote:
> On Thu, Nov 02, 2023 at 11:55:41AM -0700, Stephen Hemminger wrote:
> > On Thu,  2 Nov 2023 18:11:48 +0000 Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > 
> > > From: Bruce Richardson <bruce.richardson@intel.com> To: dev@dpdk.org
> > > Cc: stephen@networkplumber.org,  Bruce Richardson
> > > <bruce.richardson@intel.com> Subject: [PATCH] eal: add const to init
> > > function parameter Date: Thu,  2 Nov 2023 18:11:48 +0000 X-Mailer:
> > > git-send-email 2.39.2
> > > 
> > > Change the parameter type of argv parameter to rte_eal_init from
> > > "char **" to "char * const *", since we don't modify the argv
> > > pointers passed in.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > 
> > Looks good but probably needs to cascade down a few more levels
> 
> Yep. I only did a quick compile test, which passed, but I see the CI has
> thrown up a bunch more issues. I'll do a v2 later.
> 
Actually, this is not going to work as a patch. Marking as rejected in
patchwork. This is because after parsing the EAL args, we actually assign
the last arg handled, often "--", to be a copy of argv[0]. In short, we
guarantee that when we do:

	int ret = rte_eal_init(argc, argv);
	/* error handling */
	argv += ret;
	argc -= ret;

that argv[0] is the original program name set as argv[0], and so
application arg processing can be done in the usual way. Because of this,
we do modify the params, and so can't set it as an array of const pointers.

/Bruce
  

Patch

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index a6d21f1cba..7927eb1f1d 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -206,7 +206,7 @@  handle_eal_info_request(const char *cmd, const char *params __rte_unused,
 }
 
 int
-eal_save_args(int argc, char **argv)
+eal_save_args(int argc, char * const *argv)
 {
 	int i, j;
 
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb6412..21ab2492fc 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -105,7 +105,7 @@  int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
 enum rte_proc_type_t eal_proc_type_detect(void);
 int eal_plugins_init(void);
-int eal_save_args(int argc, char **argv);
+int eal_save_args(int argc, char * const *argv);
 int handle_eal_info_request(const char *cmd, const char *params __rte_unused,
 		struct rte_tel_data *d);
 
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 568e06e9ed..f241e7dd5e 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -365,7 +365,7 @@  eal_get_hugepage_mem_size(void)
 
 /* Parse the arguments for --log-level only */
 static void
-eal_log_level_parse(int argc, char **argv)
+eal_log_level_parse(int argc, char * const *argv)
 {
 	int opt;
 	char **argvopt;
@@ -577,7 +577,7 @@  static void rte_eal_init_alert(const char *msg)
 
 /* Launch threads, called at application init(). */
 int
-rte_eal_init(int argc, char **argv)
+rte_eal_init(int argc, char * const *argv)
 {
 	int i, fctret, ret;
 	static uint32_t run_once;
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index cd318ee141..e0a74865ad 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -109,7 +109,7 @@  int rte_eal_iopl_init(void);
  *
  *     ENOEXEC indicates that a service core failed to launch successfully.
  */
-int rte_eal_init(int argc, char **argv);
+int rte_eal_init(int argc, char * const *argv);
 
 /**
  * Clean up the Environment Abstraction Layer (EAL)
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 57da058cec..a56b85f2ce 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -548,7 +548,7 @@  eal_parse_vfio_vf_token(const char *vf_token)
 
 /* Parse the arguments for --log-level only */
 static void
-eal_log_level_parse(int argc, char **argv)
+eal_log_level_parse(int argc, char * const *argv)
 {
 	int opt;
 	char **argvopt;
@@ -964,7 +964,7 @@  eal_worker_thread_create(unsigned int lcore_id)
 
 /* Launch threads, called at application init(). */
 int
-rte_eal_init(int argc, char **argv)
+rte_eal_init(int argc, char * const *argv)
 {
 	int i, fctret, ret;
 	static RTE_ATOMIC(uint32_t) run_once;
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index 7ec2152211..1abb2eb2f7 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -98,7 +98,7 @@  eal_usage(const char *prgname)
 
 /* Parse the arguments for --log-level only */
 static void
-eal_log_level_parse(int argc, char **argv)
+eal_log_level_parse(int argc, char * const *argv)
 {
 	int opt;
 	char **argvopt;
@@ -273,7 +273,7 @@  rte_eal_cleanup(void)
 
 /* Launch threads, called at application init(). */
 int
-rte_eal_init(int argc, char **argv)
+rte_eal_init(int argc, char * const *argv)
 {
 	int i, fctret, bscan;
 	const struct rte_config *config = rte_eal_get_configuration();