linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] tools lib traceevent: Take care of return value of asprintf
@ 2020-02-14 13:21 zhe.he
  2020-02-17 15:53 ` Arnaldo Carvalho de Melo
  2020-02-19 19:52 ` Steven Rostedt
  0 siblings, 2 replies; 3+ messages in thread
From: zhe.he @ 2020-02-14 13:21 UTC (permalink / raw)
  To: rostedt, acme, tstoyanov, hewenliang4, linux-kernel, zhe.he

From: He Zhe <zhe.he@windriver.com>

According to the API, if memory allocation wasn't possible, or some other
error occurs, asprintf will return -1, and the contents of strp below are
undefined.

int asprintf(char **strp, const char *fmt, ...);

This patch takes care of return value of asprintf to make it less error
prone and prevent the following build warning.

ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result]

Signed-off-by: He Zhe <zhe.he@windriver.com>
---
v2: directly check the return value without saving to a variable

 tools/lib/traceevent/parse-filter.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 20eed71..6cd0228 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -274,8 +274,7 @@ find_event(struct tep_handle *tep, struct event_list **events,
 		sys_name = NULL;
 	}
 
-	ret = asprintf(&reg, "^%s$", event_name);
-	if (ret < 0)
+	if (asprintf(&reg, "^%s$", event_name) < 0)
 		return TEP_ERRNO__MEM_ALLOC_FAILED;
 
 	ret = regcomp(&ereg, reg, REG_ICASE|REG_NOSUB);
@@ -285,8 +284,7 @@ find_event(struct tep_handle *tep, struct event_list **events,
 		return TEP_ERRNO__INVALID_EVENT_NAME;
 
 	if (sys_name) {
-		ret = asprintf(&reg, "^%s$", sys_name);
-		if (ret < 0) {
+		if (asprintf(&reg, "^%s$", sys_name) < 0) {
 			regfree(&ereg);
 			return TEP_ERRNO__MEM_ALLOC_FAILED;
 		}
@@ -1958,7 +1956,8 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
 				default:
 					break;
 				}
-				asprintf(&str, val ? "TRUE" : "FALSE");
+				if (asprintf(&str, val ? "TRUE" : "FALSE") < 0)
+					str = NULL;
 				break;
 			}
 		}
@@ -1976,7 +1975,8 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
 			break;
 		}
 
-		asprintf(&str, "(%s) %s (%s)", left, op, right);
+		if (asprintf(&str, "(%s) %s (%s)", left, op, right) < 0)
+			str = NULL;
 		break;
 
 	case TEP_FILTER_OP_NOT:
@@ -1992,10 +1992,12 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
 			right_val = 0;
 		if (right_val >= 0) {
 			/* just return the opposite */
-			asprintf(&str, right_val ? "FALSE" : "TRUE");
+			if (asprintf(&str, right_val ? "FALSE" : "TRUE") < 0)
+				str = NULL;
 			break;
 		}
-		asprintf(&str, "%s(%s)", op, right);
+		if (asprintf(&str, "%s(%s)", op, right) < 0)
+			str = NULL;
 		break;
 
 	default:
@@ -2011,7 +2013,8 @@ static char *val_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
 {
 	char *str = NULL;
 
-	asprintf(&str, "%lld", arg->value.val);
+	if (asprintf(&str, "%lld", arg->value.val) < 0)
+		str = NULL;
 
 	return str;
 }
@@ -2069,7 +2072,8 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
 		break;
 	}
 
-	asprintf(&str, "%s %s %s", lstr, op, rstr);
+	if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0)
+		str = NULL;
 out:
 	free(lstr);
 	free(rstr);
@@ -2113,7 +2117,8 @@ static char *num_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
 		if (!op)
 			op = "<=";
 
-		asprintf(&str, "%s %s %s", lstr, op, rstr);
+		if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0)
+			str = NULL;
 		break;
 
 	default:
@@ -2148,8 +2153,9 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
 		if (!op)
 			op = "!~";
 
-		asprintf(&str, "%s %s \"%s\"",
-			 arg->str.field->name, op, arg->str.val);
+		if (asprintf(&str, "%s %s \"%s\"",
+			 arg->str.field->name, op, arg->str.val) < 0)
+			str = NULL;
 		break;
 
 	default:
@@ -2165,7 +2171,8 @@ static char *arg_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
 
 	switch (arg->type) {
 	case TEP_FILTER_ARG_BOOLEAN:
-		asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
+		if (asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE") < 0)
+			str = NULL;
 		return str;
 
 	case TEP_FILTER_ARG_OP:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] tools lib traceevent: Take care of return value of asprintf
  2020-02-14 13:21 [PATCH v2] tools lib traceevent: Take care of return value of asprintf zhe.he
@ 2020-02-17 15:53 ` Arnaldo Carvalho de Melo
  2020-02-19 19:52 ` Steven Rostedt
  1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-17 15:53 UTC (permalink / raw)
  To: Steven Rostedt, zhe.he
  Cc: rostedt, tstoyanov, hewenliang4, linux-kernel, acme

Em Fri, Feb 14, 2020 at 09:21:21PM +0800, zhe.he@windriver.com escreveu:
> From: He Zhe <zhe.he@windriver.com>
> 
> According to the API, if memory allocation wasn't possible, or some other
> error occurs, asprintf will return -1, and the contents of strp below are
> undefined.

Rostedt, did I miss some ack from you?

- Arnaldo
 
> int asprintf(char **strp, const char *fmt, ...);
> 
> This patch takes care of return value of asprintf to make it less error
> prone and prevent the following build warning.
> 
> ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result]
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> v2: directly check the return value without saving to a variable
> 
>  tools/lib/traceevent/parse-filter.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
> index 20eed71..6cd0228 100644
> --- a/tools/lib/traceevent/parse-filter.c
> +++ b/tools/lib/traceevent/parse-filter.c
> @@ -274,8 +274,7 @@ find_event(struct tep_handle *tep, struct event_list **events,
>  		sys_name = NULL;
>  	}
>  
> -	ret = asprintf(&reg, "^%s$", event_name);
> -	if (ret < 0)
> +	if (asprintf(&reg, "^%s$", event_name) < 0)
>  		return TEP_ERRNO__MEM_ALLOC_FAILED;
>  
>  	ret = regcomp(&ereg, reg, REG_ICASE|REG_NOSUB);
> @@ -285,8 +284,7 @@ find_event(struct tep_handle *tep, struct event_list **events,
>  		return TEP_ERRNO__INVALID_EVENT_NAME;
>  
>  	if (sys_name) {
> -		ret = asprintf(&reg, "^%s$", sys_name);
> -		if (ret < 0) {
> +		if (asprintf(&reg, "^%s$", sys_name) < 0) {
>  			regfree(&ereg);
>  			return TEP_ERRNO__MEM_ALLOC_FAILED;
>  		}
> @@ -1958,7 +1956,8 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>  				default:
>  					break;
>  				}
> -				asprintf(&str, val ? "TRUE" : "FALSE");
> +				if (asprintf(&str, val ? "TRUE" : "FALSE") < 0)
> +					str = NULL;
>  				break;
>  			}
>  		}
> @@ -1976,7 +1975,8 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>  			break;
>  		}
>  
> -		asprintf(&str, "(%s) %s (%s)", left, op, right);
> +		if (asprintf(&str, "(%s) %s (%s)", left, op, right) < 0)
> +			str = NULL;
>  		break;
>  
>  	case TEP_FILTER_OP_NOT:
> @@ -1992,10 +1992,12 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>  			right_val = 0;
>  		if (right_val >= 0) {
>  			/* just return the opposite */
> -			asprintf(&str, right_val ? "FALSE" : "TRUE");
> +			if (asprintf(&str, right_val ? "FALSE" : "TRUE") < 0)
> +				str = NULL;
>  			break;
>  		}
> -		asprintf(&str, "%s(%s)", op, right);
> +		if (asprintf(&str, "%s(%s)", op, right) < 0)
> +			str = NULL;
>  		break;
>  
>  	default:
> @@ -2011,7 +2013,8 @@ static char *val_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>  {
>  	char *str = NULL;
>  
> -	asprintf(&str, "%lld", arg->value.val);
> +	if (asprintf(&str, "%lld", arg->value.val) < 0)
> +		str = NULL;
>  
>  	return str;
>  }
> @@ -2069,7 +2072,8 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>  		break;
>  	}
>  
> -	asprintf(&str, "%s %s %s", lstr, op, rstr);
> +	if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0)
> +		str = NULL;
>  out:
>  	free(lstr);
>  	free(rstr);
> @@ -2113,7 +2117,8 @@ static char *num_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>  		if (!op)
>  			op = "<=";
>  
> -		asprintf(&str, "%s %s %s", lstr, op, rstr);
> +		if (asprintf(&str, "%s %s %s", lstr, op, rstr) < 0)
> +			str = NULL;
>  		break;
>  
>  	default:
> @@ -2148,8 +2153,9 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>  		if (!op)
>  			op = "!~";
>  
> -		asprintf(&str, "%s %s \"%s\"",
> -			 arg->str.field->name, op, arg->str.val);
> +		if (asprintf(&str, "%s %s \"%s\"",
> +			 arg->str.field->name, op, arg->str.val) < 0)
> +			str = NULL;
>  		break;
>  
>  	default:
> @@ -2165,7 +2171,8 @@ static char *arg_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>  
>  	switch (arg->type) {
>  	case TEP_FILTER_ARG_BOOLEAN:
> -		asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
> +		if (asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE") < 0)
> +			str = NULL;
>  		return str;
>  
>  	case TEP_FILTER_ARG_OP:
> -- 
> 2.7.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] tools lib traceevent: Take care of return value of asprintf
  2020-02-14 13:21 [PATCH v2] tools lib traceevent: Take care of return value of asprintf zhe.he
  2020-02-17 15:53 ` Arnaldo Carvalho de Melo
@ 2020-02-19 19:52 ` Steven Rostedt
  1 sibling, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2020-02-19 19:52 UTC (permalink / raw)
  To: zhe.he; +Cc: acme, tstoyanov, hewenliang4, linux-kernel

On Fri, 14 Feb 2020 21:21:21 +0800
<zhe.he@windriver.com> wrote:

> From: He Zhe <zhe.he@windriver.com>
> 
> According to the API, if memory allocation wasn't possible, or some other
> error occurs, asprintf will return -1, and the contents of strp below are
> undefined.
> 
> int asprintf(char **strp, const char *fmt, ...);
> 
> This patch takes care of return value of asprintf to make it less error
> prone and prevent the following build warning.
> 
> ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result]
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> v2: directly check the return value without saving to a variable
> 
>  tools/lib/traceevent/parse-filter.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
> index 20eed71..6cd0228 100644
> --- a/tools/lib/traceevent/parse-filter.c
> +++ b/tools/lib/traceevent/parse-filter.c
> @@ -274,8 +274,7 @@ find_event(struct tep_handle *tep, struct event_list **events,
>  		sys_name = NULL;
>  	}
>  
> -	ret = asprintf(&reg, "^%s$", event_name);
> -	if (ret < 0)
> +	if (asprintf(&reg, "^%s$", event_name) < 0)

I know Arnaldo said you don't need to save the return value for the
check, but let's just modify the asprintf() that needs checking, and
not remove those that already save it.

Thanks!

-- Steve


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-02-19 19:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 13:21 [PATCH v2] tools lib traceevent: Take care of return value of asprintf zhe.he
2020-02-17 15:53 ` Arnaldo Carvalho de Melo
2020-02-19 19:52 ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).