* [PATCH] tools lib traceevent: Take care of return value of asprintf
@ 2020-02-14 7:31 zhe.he
2020-02-14 12:57 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: zhe.he @ 2020-02-14 7:31 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>
---
tools/lib/traceevent/parse-filter.c | 42 +++++++++++++++++++++++++++++--------
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 20eed71..279b572 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -1912,6 +1912,7 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
int left_val = -1;
int right_val = -1;
int val;
+ int ret;
switch (arg->op.type) {
case TEP_FILTER_OP_AND:
@@ -1958,7 +1959,9 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
default:
break;
}
- asprintf(&str, val ? "TRUE" : "FALSE");
+ ret = asprintf(&str, val ? "TRUE" : "FALSE");
+ if (ret < 0)
+ str = NULL;
break;
}
}
@@ -1976,7 +1979,9 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
break;
}
- asprintf(&str, "(%s) %s (%s)", left, op, right);
+ ret = asprintf(&str, "(%s) %s (%s)", left, op, right);
+ if (ret < 0)
+ str = NULL;
break;
case TEP_FILTER_OP_NOT:
@@ -1992,10 +1997,14 @@ 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");
+ ret = asprintf(&str, right_val ? "FALSE" : "TRUE");
+ if (ret < 0)
+ str = NULL;
break;
}
- asprintf(&str, "%s(%s)", op, right);
+ ret = asprintf(&str, "%s(%s)", op, right);
+ if (ret < 0)
+ str = NULL;
break;
default:
@@ -2010,8 +2019,11 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
static char *val_to_str(struct tep_event_filter *filter, struct tep_filter_arg *arg)
{
char *str = NULL;
+ int ret;
- asprintf(&str, "%lld", arg->value.val);
+ ret = asprintf(&str, "%lld", arg->value.val);
+ if (ret < 0)
+ str = NULL;
return str;
}
@@ -2027,6 +2039,7 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
char *rstr;
char *op;
char *str = NULL;
+ int ret;
lstr = arg_to_str(filter, arg->exp.left);
rstr = arg_to_str(filter, arg->exp.right);
@@ -2069,7 +2082,9 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
break;
}
- asprintf(&str, "%s %s %s", lstr, op, rstr);
+ ret = asprintf(&str, "%s %s %s", lstr, op, rstr);
+ if (ret < 0)
+ str = NULL;
out:
free(lstr);
free(rstr);
@@ -2083,6 +2098,7 @@ static char *num_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
char *rstr;
char *str = NULL;
char *op = NULL;
+ int ret;
lstr = arg_to_str(filter, arg->num.left);
rstr = arg_to_str(filter, arg->num.right);
@@ -2113,7 +2129,9 @@ 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);
+ ret = asprintf(&str, "%s %s %s", lstr, op, rstr);
+ if (ret < 0)
+ str = NULL;
break;
default:
@@ -2131,6 +2149,7 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
{
char *str = NULL;
char *op = NULL;
+ int ret;
switch (arg->str.type) {
case TEP_FILTER_CMP_MATCH:
@@ -2148,8 +2167,10 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
if (!op)
op = "!~";
- asprintf(&str, "%s %s \"%s\"",
+ ret = asprintf(&str, "%s %s \"%s\"",
arg->str.field->name, op, arg->str.val);
+ if (ret < 0)
+ str = NULL;
break;
default:
@@ -2162,10 +2183,13 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
static char *arg_to_str(struct tep_event_filter *filter, struct tep_filter_arg *arg)
{
char *str = NULL;
+ int ret;
switch (arg->type) {
case TEP_FILTER_ARG_BOOLEAN:
- asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
+ ret = asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
+ if (ret < 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] tools lib traceevent: Take care of return value of asprintf
2020-02-14 7:31 [PATCH] tools lib traceevent: Take care of return value of asprintf zhe.he
@ 2020-02-14 12:57 ` Arnaldo Carvalho de Melo
2020-02-14 13:17 ` He Zhe
0 siblings, 1 reply; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-14 12:57 UTC (permalink / raw)
To: zhe.he; +Cc: rostedt, tstoyanov, hewenliang4, linux-kernel
Em Fri, Feb 14, 2020 at 03:31:26PM +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.
>
> 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]
Sure this fixes problems, but can you make it more compact, i.e. no need
to add most if not all those 'int ret;' lines, just check the asprintf
result directly, i.e.:
if (asprintf(&str, val ? "TRUE" : "FALSE") < 0)
str = NULL;
saving the return value for that function is interesting when you need
to know how many bytes it printed to do some extra calculation, but if
all you need is to know if it failed, checking against < 0 is enough,
no?
- Arnaldo
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
> tools/lib/traceevent/parse-filter.c | 42 +++++++++++++++++++++++++++++--------
> 1 file changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
> index 20eed71..279b572 100644
> --- a/tools/lib/traceevent/parse-filter.c
> +++ b/tools/lib/traceevent/parse-filter.c
> @@ -1912,6 +1912,7 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
> int left_val = -1;
> int right_val = -1;
> int val;
> + int ret;
>
> switch (arg->op.type) {
> case TEP_FILTER_OP_AND:
> @@ -1958,7 +1959,9 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
> default:
> break;
> }
> - asprintf(&str, val ? "TRUE" : "FALSE");
> + ret = asprintf(&str, val ? "TRUE" : "FALSE");
> + if (ret < 0)
> + str = NULL;
> break;
> }
> }
> @@ -1976,7 +1979,9 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
> break;
> }
>
> - asprintf(&str, "(%s) %s (%s)", left, op, right);
> + ret = asprintf(&str, "(%s) %s (%s)", left, op, right);
> + if (ret < 0)
> + str = NULL;
> break;
>
> case TEP_FILTER_OP_NOT:
> @@ -1992,10 +1997,14 @@ 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");
> + ret = asprintf(&str, right_val ? "FALSE" : "TRUE");
> + if (ret < 0)
> + str = NULL;
> break;
> }
> - asprintf(&str, "%s(%s)", op, right);
> + ret = asprintf(&str, "%s(%s)", op, right);
> + if (ret < 0)
> + str = NULL;
> break;
>
> default:
> @@ -2010,8 +2019,11 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
> static char *val_to_str(struct tep_event_filter *filter, struct tep_filter_arg *arg)
> {
> char *str = NULL;
> + int ret;
>
> - asprintf(&str, "%lld", arg->value.val);
> + ret = asprintf(&str, "%lld", arg->value.val);
> + if (ret < 0)
> + str = NULL;
>
> return str;
> }
> @@ -2027,6 +2039,7 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
> char *rstr;
> char *op;
> char *str = NULL;
> + int ret;
>
> lstr = arg_to_str(filter, arg->exp.left);
> rstr = arg_to_str(filter, arg->exp.right);
> @@ -2069,7 +2082,9 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
> break;
> }
>
> - asprintf(&str, "%s %s %s", lstr, op, rstr);
> + ret = asprintf(&str, "%s %s %s", lstr, op, rstr);
> + if (ret < 0)
> + str = NULL;
> out:
> free(lstr);
> free(rstr);
> @@ -2083,6 +2098,7 @@ static char *num_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
> char *rstr;
> char *str = NULL;
> char *op = NULL;
> + int ret;
>
> lstr = arg_to_str(filter, arg->num.left);
> rstr = arg_to_str(filter, arg->num.right);
> @@ -2113,7 +2129,9 @@ 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);
> + ret = asprintf(&str, "%s %s %s", lstr, op, rstr);
> + if (ret < 0)
> + str = NULL;
> break;
>
> default:
> @@ -2131,6 +2149,7 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
> {
> char *str = NULL;
> char *op = NULL;
> + int ret;
>
> switch (arg->str.type) {
> case TEP_FILTER_CMP_MATCH:
> @@ -2148,8 +2167,10 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
> if (!op)
> op = "!~";
>
> - asprintf(&str, "%s %s \"%s\"",
> + ret = asprintf(&str, "%s %s \"%s\"",
> arg->str.field->name, op, arg->str.val);
> + if (ret < 0)
> + str = NULL;
> break;
>
> default:
> @@ -2162,10 +2183,13 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
> static char *arg_to_str(struct tep_event_filter *filter, struct tep_filter_arg *arg)
> {
> char *str = NULL;
> + int ret;
>
> switch (arg->type) {
> case TEP_FILTER_ARG_BOOLEAN:
> - asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
> + ret = asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
> + if (ret < 0)
> + str = NULL;
> return str;
>
> case TEP_FILTER_ARG_OP:
> --
> 2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tools lib traceevent: Take care of return value of asprintf
2020-02-14 12:57 ` Arnaldo Carvalho de Melo
@ 2020-02-14 13:17 ` He Zhe
0 siblings, 0 replies; 3+ messages in thread
From: He Zhe @ 2020-02-14 13:17 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: rostedt, tstoyanov, hewenliang4, linux-kernel
On 2/14/20 8:57 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 14, 2020 at 03:31:26PM +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.
>>
>> 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]
> Sure this fixes problems, but can you make it more compact, i.e. no need
> to add most if not all those 'int ret;' lines, just check the asprintf
> result directly, i.e.:
>
> if (asprintf(&str, val ? "TRUE" : "FALSE") < 0)
> str = NULL;
>
> saving the return value for that function is interesting when you need
> to know how many bytes it printed to do some extra calculation, but if
> all you need is to know if it failed, checking against < 0 is enough,
> no?
OK. Thanks. I'll send v2.
Zhe
>
> - Arnaldo
>
>
>> Signed-off-by: He Zhe <zhe.he@windriver.com>
>> ---
>> tools/lib/traceevent/parse-filter.c | 42 +++++++++++++++++++++++++++++--------
>> 1 file changed, 33 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
>> index 20eed71..279b572 100644
>> --- a/tools/lib/traceevent/parse-filter.c
>> +++ b/tools/lib/traceevent/parse-filter.c
>> @@ -1912,6 +1912,7 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>> int left_val = -1;
>> int right_val = -1;
>> int val;
>> + int ret;
>>
>> switch (arg->op.type) {
>> case TEP_FILTER_OP_AND:
>> @@ -1958,7 +1959,9 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>> default:
>> break;
>> }
>> - asprintf(&str, val ? "TRUE" : "FALSE");
>> + ret = asprintf(&str, val ? "TRUE" : "FALSE");
>> + if (ret < 0)
>> + str = NULL;
>> break;
>> }
>> }
>> @@ -1976,7 +1979,9 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>> break;
>> }
>>
>> - asprintf(&str, "(%s) %s (%s)", left, op, right);
>> + ret = asprintf(&str, "(%s) %s (%s)", left, op, right);
>> + if (ret < 0)
>> + str = NULL;
>> break;
>>
>> case TEP_FILTER_OP_NOT:
>> @@ -1992,10 +1997,14 @@ 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");
>> + ret = asprintf(&str, right_val ? "FALSE" : "TRUE");
>> + if (ret < 0)
>> + str = NULL;
>> break;
>> }
>> - asprintf(&str, "%s(%s)", op, right);
>> + ret = asprintf(&str, "%s(%s)", op, right);
>> + if (ret < 0)
>> + str = NULL;
>> break;
>>
>> default:
>> @@ -2010,8 +2019,11 @@ static char *op_to_str(struct tep_event_filter *filter, struct tep_filter_arg *a
>> static char *val_to_str(struct tep_event_filter *filter, struct tep_filter_arg *arg)
>> {
>> char *str = NULL;
>> + int ret;
>>
>> - asprintf(&str, "%lld", arg->value.val);
>> + ret = asprintf(&str, "%lld", arg->value.val);
>> + if (ret < 0)
>> + str = NULL;
>>
>> return str;
>> }
>> @@ -2027,6 +2039,7 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>> char *rstr;
>> char *op;
>> char *str = NULL;
>> + int ret;
>>
>> lstr = arg_to_str(filter, arg->exp.left);
>> rstr = arg_to_str(filter, arg->exp.right);
>> @@ -2069,7 +2082,9 @@ static char *exp_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>> break;
>> }
>>
>> - asprintf(&str, "%s %s %s", lstr, op, rstr);
>> + ret = asprintf(&str, "%s %s %s", lstr, op, rstr);
>> + if (ret < 0)
>> + str = NULL;
>> out:
>> free(lstr);
>> free(rstr);
>> @@ -2083,6 +2098,7 @@ static char *num_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>> char *rstr;
>> char *str = NULL;
>> char *op = NULL;
>> + int ret;
>>
>> lstr = arg_to_str(filter, arg->num.left);
>> rstr = arg_to_str(filter, arg->num.right);
>> @@ -2113,7 +2129,9 @@ 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);
>> + ret = asprintf(&str, "%s %s %s", lstr, op, rstr);
>> + if (ret < 0)
>> + str = NULL;
>> break;
>>
>> default:
>> @@ -2131,6 +2149,7 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>> {
>> char *str = NULL;
>> char *op = NULL;
>> + int ret;
>>
>> switch (arg->str.type) {
>> case TEP_FILTER_CMP_MATCH:
>> @@ -2148,8 +2167,10 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>> if (!op)
>> op = "!~";
>>
>> - asprintf(&str, "%s %s \"%s\"",
>> + ret = asprintf(&str, "%s %s \"%s\"",
>> arg->str.field->name, op, arg->str.val);
>> + if (ret < 0)
>> + str = NULL;
>> break;
>>
>> default:
>> @@ -2162,10 +2183,13 @@ static char *str_to_str(struct tep_event_filter *filter, struct tep_filter_arg *
>> static char *arg_to_str(struct tep_event_filter *filter, struct tep_filter_arg *arg)
>> {
>> char *str = NULL;
>> + int ret;
>>
>> switch (arg->type) {
>> case TEP_FILTER_ARG_BOOLEAN:
>> - asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
>> + ret = asprintf(&str, arg->boolean.value ? "TRUE" : "FALSE");
>> + if (ret < 0)
>> + str = NULL;
>> return str;
>>
>> case TEP_FILTER_ARG_OP:
>> --
>> 2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-14 13:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 7:31 [PATCH] tools lib traceevent: Take care of return value of asprintf zhe.he
2020-02-14 12:57 ` Arnaldo Carvalho de Melo
2020-02-14 13:17 ` He Zhe
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).