[4/8] trace: avoid confusion on optarg

Message ID 20200503203135.6493-5-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series Traces cleanup for rc2 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

David Marchand May 3, 2020, 8:31 p.m. UTC
  Prefer a local name to optarg which is a global symbol from the C library.

Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .../common/eal_common_trace_utils.c           | 32 +++++++++----------
 lib/librte_eal/common/eal_trace.h             |  8 ++---
 2 files changed, 20 insertions(+), 20 deletions(-)
  

Comments

Sunil Kumar Kori May 4, 2020, 7:55 a.m. UTC | #1
Overall, it looks okay but I think "args" will be more relevant as each API says XXX_args_save().
What do you say ?

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Monday, May 4, 2020 2:02 AM
>To: dev@dpdk.org
>Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>Sunil Kumar Kori <skori@marvell.com>
>Subject: [EXT] [PATCH 4/8] trace: avoid confusion on optarg
>
>External Email
>
>----------------------------------------------------------------------
>Prefer a local name to optarg which is a global symbol from the C library.
>
>Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter")
>
>Signed-off-by: David Marchand <david.marchand@redhat.com>
>---
> .../common/eal_common_trace_utils.c           | 32 +++++++++----------
> lib/librte_eal/common/eal_trace.h             |  8 ++---
> 2 files changed, 20 insertions(+), 20 deletions(-)
>
>diff --git a/lib/librte_eal/common/eal_common_trace_utils.c
>b/lib/librte_eal/common/eal_common_trace_utils.c
>index fce8892c38..a7c5893b00 100644
>--- a/lib/librte_eal/common/eal_common_trace_utils.c
>+++ b/lib/librte_eal/common/eal_common_trace_utils.c
>@@ -135,7 +135,7 @@ trace_dir_update(const char *str)  }
>
> int
>-eal_trace_args_save(const char *optarg)
>+eal_trace_args_save(const char *val)
> {
> 	struct trace *trace = trace_obj_get();
> 	char *trace_args;
>@@ -144,17 +144,17 @@ eal_trace_args_save(const char *optarg)
> 	nb_args = trace->args.nb_args;
>
> 	if (nb_args >= TRACE_MAX_ARGS) {
>-		trace_err("ignoring trace %s as limit exceeds", optarg);
>+		trace_err("ignoring trace %s as limit exceeds", val);
> 		return 0;
> 	}
>
>-	trace_args = calloc(1, (strlen(optarg) + 1));
>+	trace_args = calloc(1, (strlen(val) + 1));
> 	if (trace_args == NULL) {
>-		trace_err("fail to allocate memory for %s", optarg);
>+		trace_err("fail to allocate memory for %s", val);
> 		return -ENOMEM;
> 	}
>
>-	memcpy(trace_args, optarg, strlen(optarg));
>+	memcpy(trace_args, val, strlen(val));
> 	trace->args.args[nb_args++] = trace_args;
> 	trace->args.nb_args = nb_args;
> 	return 0;
[snip]
>2.23.0
  
David Marchand May 4, 2020, 2:09 p.m. UTC | #2
On Mon, May 4, 2020 at 9:55 AM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> Overall, it looks okay but I think "args" will be more relevant as each API says XXX_args_save().
> What do you say ?

No opinion, the function name itself indicates we are dealing with arguments.
I can go with args if you like.
  
Sunil Kumar Kori May 5, 2020, 5:45 a.m. UTC | #3
>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Monday, May 4, 2020 7:39 PM
>To: Sunil Kumar Kori <skori@marvell.com>
>Cc: dev@dpdk.org; thomas@monjalon.net; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Subject: Re: [EXT] [PATCH 4/8] trace: avoid confusion on optarg
>
>On Mon, May 4, 2020 at 9:55 AM Sunil Kumar Kori <skori@marvell.com>
>wrote:
>>
>> Overall, it looks okay but I think "args" will be more relevant as each API says
>XXX_args_save().
>> What do you say ?
>
>No opinion, the function name itself indicates we are dealing with arguments.
>I can go with args if you like.
No problem. Let it be as it is. 

>
>
>--
>David Marchand
  
Sunil Kumar Kori May 5, 2020, 5:47 a.m. UTC | #4
>-----Original Message-----
>From: Sunil Kumar Kori
>Sent: Tuesday, May 5, 2020 11:15 AM
>To: David Marchand <david.marchand@redhat.com>
>Cc: dev@dpdk.org; thomas@monjalon.net; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Subject: RE: [EXT] [PATCH 4/8] trace: avoid confusion on optarg
>
>>-----Original Message-----
>>From: David Marchand <david.marchand@redhat.com>
>>Sent: Monday, May 4, 2020 7:39 PM
>>To: Sunil Kumar Kori <skori@marvell.com>
>>Cc: dev@dpdk.org; thomas@monjalon.net; Jerin Jacob Kollanukkaran
>><jerinj@marvell.com>
>>Subject: Re: [EXT] [PATCH 4/8] trace: avoid confusion on optarg
>>
>>On Mon, May 4, 2020 at 9:55 AM Sunil Kumar Kori <skori@marvell.com>
>>wrote:
>>>
>>> Overall, it looks okay but I think "args" will be more relevant as
>>> each API says
>>XXX_args_save().
>>> What do you say ?
>>
>>No opinion, the function name itself indicates we are dealing with
>arguments.
>>I can go with args if you like.
>No problem. Let it be as it is.
>
Acked-by: Sunil Kumar Kori <skori@mavell.com>

>>
>>
>>--
>>David Marchand
  

Patch

diff --git a/lib/librte_eal/common/eal_common_trace_utils.c b/lib/librte_eal/common/eal_common_trace_utils.c
index fce8892c38..a7c5893b00 100644
--- a/lib/librte_eal/common/eal_common_trace_utils.c
+++ b/lib/librte_eal/common/eal_common_trace_utils.c
@@ -135,7 +135,7 @@  trace_dir_update(const char *str)
 }
 
 int
-eal_trace_args_save(const char *optarg)
+eal_trace_args_save(const char *val)
 {
 	struct trace *trace = trace_obj_get();
 	char *trace_args;
@@ -144,17 +144,17 @@  eal_trace_args_save(const char *optarg)
 	nb_args = trace->args.nb_args;
 
 	if (nb_args >= TRACE_MAX_ARGS) {
-		trace_err("ignoring trace %s as limit exceeds", optarg);
+		trace_err("ignoring trace %s as limit exceeds", val);
 		return 0;
 	}
 
-	trace_args = calloc(1, (strlen(optarg) + 1));
+	trace_args = calloc(1, (strlen(val) + 1));
 	if (trace_args == NULL) {
-		trace_err("fail to allocate memory for %s", optarg);
+		trace_err("fail to allocate memory for %s", val);
 		return -ENOMEM;
 	}
 
-	memcpy(trace_args, optarg, strlen(optarg));
+	memcpy(trace_args, val, strlen(val));
 	trace->args.args[nb_args++] = trace_args;
 	trace->args.nb_args = nb_args;
 	return 0;
@@ -194,17 +194,17 @@  trace_args_apply(const char *arg)
 }
 
 int
-eal_trace_bufsz_args_save(char const *optarg)
+eal_trace_bufsz_args_save(char const *val)
 {
 	struct trace *trace = trace_obj_get();
 	uint64_t bufsz;
 
-	if (optarg == NULL) {
+	if (val == NULL) {
 		trace_err("no optarg is passed");
 		return -EINVAL;
 	}
 
-	bufsz = rte_str_to_size(optarg);
+	bufsz = rte_str_to_size(val);
 	if (bufsz == 0) {
 		trace_err("buffer size cannot be zero");
 		return -EINVAL;
@@ -224,14 +224,14 @@  trace_bufsz_args_apply(void)
 }
 
 int
-eal_trace_mode_args_save(const char *optarg)
+eal_trace_mode_args_save(const char *val)
 {
 	struct trace *trace = trace_obj_get();
-	size_t len = strlen(optarg);
+	size_t len = strlen(val);
 	unsigned long tmp;
 	char *pattern;
 
-	if (optarg == NULL) {
+	if (val == NULL) {
 		trace_err("no optarg is passed");
 		return -EINVAL;
 	}
@@ -247,7 +247,7 @@  eal_trace_mode_args_save(const char *optarg)
 		return -ENOMEM;
 	}
 
-	sprintf(pattern, "%s*", optarg);
+	sprintf(pattern, "%s*", val);
 
 	if (fnmatch(pattern, "overwrite", 0) == 0)
 		tmp = RTE_TRACE_MODE_OVERWRITE;
@@ -264,19 +264,19 @@  eal_trace_mode_args_save(const char *optarg)
 }
 
 int
-eal_trace_dir_args_save(char const *optarg)
+eal_trace_dir_args_save(char const *val)
 {
 	struct trace *trace = trace_obj_get();
 	uint32_t size = sizeof(trace->dir);
 	char *dir_path = NULL;
 	int rc;
 
-	if (optarg == NULL) {
+	if (val == NULL) {
 		trace_err("no optarg is passed");
 		return -EINVAL;
 	}
 
-	if (strlen(optarg) >= size) {
+	if (strlen(val) >= size) {
 		trace_err("input string is too big");
 		return -ENAMETOOLONG;
 	}
@@ -287,7 +287,7 @@  eal_trace_dir_args_save(char const *optarg)
 		return -ENOMEM;
 	}
 
-	sprintf(dir_path, "%s/", optarg);
+	sprintf(dir_path, "%s/", val);
 	rc = trace_dir_update(dir_path);
 
 	free(dir_path);
diff --git a/lib/librte_eal/common/eal_trace.h b/lib/librte_eal/common/eal_trace.h
index f53ccc0d50..7d95bd2aa9 100644
--- a/lib/librte_eal/common/eal_trace.h
+++ b/lib/librte_eal/common/eal_trace.h
@@ -111,10 +111,10 @@  void trace_mem_per_thread_free(void);
 /* EAL interface */
 int eal_trace_init(void);
 void eal_trace_fini(void);
-int eal_trace_args_save(const char *optarg);
+int eal_trace_args_save(const char *val);
 void eal_trace_args_free(void);
-int eal_trace_dir_args_save(const char *optarg);
-int eal_trace_mode_args_save(const char *optarg);
-int eal_trace_bufsz_args_save(const char *optarg);
+int eal_trace_dir_args_save(const char *val);
+int eal_trace_mode_args_save(const char *val);
+int eal_trace_bufsz_args_save(const char *val);
 
 #endif /* __EAL_TRACE_H */