| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20131213145204.GC29275@ghostprotocols.net>
Date: Fri, 13 Dec 2013 11:52:04 -0300
From: Arnaldo Carvalho de Melo <acme@...stprotocols.net>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Ingo Molnar <mingo@...nel.org>,
Namhyung Kim <namhyung.kim@....com>,
LKML <linux-kernel@...r.kernel.org>, Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH 13/14] tools lib traceevent: Get rid of die() in some
string conversion funcitons
Em Fri, Dec 13, 2013 at 09:15:46AM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
>
> On Thu, 12 Dec 2013 15:41:47 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Dec 12, 2013 at 04:36:16PM +0900, Namhyung Kim escreveu:
> >> Those functions are for stringify filter arguments. As caller of
> >> those functions handles NULL string properly, it seems that it's
> >> enough to return NULL rather than calling die().
> >>
> >> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> >> ---
> >> tools/lib/traceevent/parse-filter.c | 58 +++++++++++++++++++++++--------------
> >> 1 file changed, 36 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
> >> index 9303c55128db..32ab4396653c 100644
> >> --- a/tools/lib/traceevent/parse-filter.c
> >> +++ b/tools/lib/traceevent/parse-filter.c
> >> @@ -1361,8 +1361,10 @@ enum pevent_errno pevent_filter_add_filter_str(struct event_filter *filter,
> >> if (ret >= 0 && pevent->test_filters) {
> >> char *test;
> >> test = pevent_filter_make_string(filter, event->event->id);
> >> - printf(" '%s: %s'\n", event->event->name, test);
> >> - free(test);
> >> + if (test) {
> >> + printf(" '%s: %s'\n", event->event->name, test);
> >> + free(test);
> >> + }
> >> }
> >> }
> >>
> >> @@ -2097,7 +2099,9 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)
> >> default:
> >> break;
> >> }
> >> - str = malloc_or_die(6);
> >> + str = malloc(6);
> >> + if (str == NULL)
> >> + break;
> >> if (val)
> >> strcpy(str, "TRUE");
> >> else
> >> @@ -2120,7 +2124,9 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)
> >> }
> >>
> >> len = strlen(left) + strlen(right) + strlen(op) + 10;
> >> - str = malloc_or_die(len);
> >> + str = malloc(len);
> >> + if (str == NULL)
> >> + break;
> >> snprintf(str, len, "(%s) %s (%s)",
> >> left, op, right);
> >
> > Why note:
> >
> > str = NULL;
> > if (asprintf(&str, "(%s) %s (%s)", left, op, right);
> > break;
> >
> > instead?
> >
> > Ditto for other places here, also there is a typo in the subject line.
> >
> > I've applied all the previous patches, thanks!
>
> Thank you very much!
>
> Here is the updated patch:
Thanks! There are some more questions below:
>
> From d775295fb53930b1e3d946e145e9a6172773d4f5 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung@...nel.org>
> Date: Sat, 7 Dec 2013 15:13:35 +0000
> Subject: [PATCH v2.1 13/14] tools lib traceevent: Get rid of die() in some string
> conversion functions
>
> Those functions are for stringify filter arguments. As caller of
> those functions handles NULL string properly, it seems that it's
> enough to return NULL rather than calling die().
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/lib/traceevent/parse-filter.c | 59 +++++++++++++++----------------------
> 1 file changed, 24 insertions(+), 35 deletions(-)
>
> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
> index 9303c55128db..72fe16562546 100644
> --- a/tools/lib/traceevent/parse-filter.c
> +++ b/tools/lib/traceevent/parse-filter.c
> @@ -1361,8 +1361,10 @@ enum pevent_errno pevent_filter_add_filter_str(struct event_filter *filter,
> if (ret >= 0 && pevent->test_filters) {
> char *test;
> test = pevent_filter_make_string(filter, event->event->id);
> - printf(" '%s: %s'\n", event->event->name, test);
> - free(test);
> + if (test) {
> + printf(" '%s: %s'\n", event->event->name, test);
> + free(test);
> + }
> }
> }
>
> @@ -2050,7 +2052,6 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)
> int left_val = -1;
> int right_val = -1;
> int val;
> - int len;
>
> switch (arg->op.type) {
> case FILTER_OP_AND:
> @@ -2097,7 +2098,9 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)
> default:
> break;
> }
> - str = malloc_or_die(6);
> + str = malloc(6);
> + if (str == NULL)
> + break;
> if (val)
> strcpy(str, "TRUE");
> else
The malloc here can be combined with the strcpy, and the else clause
gets immediately after:
if (asprintf(&str, "%s", "TRUE") < 0)
> @@ -2119,10 +2122,7 @@ static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)
> break;
> }
>
> - len = strlen(left) + strlen(right) + strlen(op) + 10;
> - str = malloc_or_die(len);
> - snprintf(str, len, "(%s) %s (%s)",
> - left, op, right);
> + asprintf(&str, "(%s) %s (%s)", left, op, right);
> break;
I was unsure if str was NULL, it is already, at decl site, good :)
All the rest is ok, so its just the malloc + strcpy that remains to be
converted, do you want me to do it?
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists