On Mon, 9 Dec 2013 11:44:37 +0100, Jiri Olsa wrote: > On Mon, Dec 09, 2013 at 02:33:59PM +0900, Namhyung Kim wrote: >> The realloc() should check return value and not to overwrite previous >> pointer in case of error. >> >> Signed-off-by: Namhyung Kim <[email protected]> >> --- >> tools/lib/traceevent/parse-filter.c | 21 ++++++++++++++++----- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/tools/lib/traceevent/parse-filter.c >> b/tools/lib/traceevent/parse-filter.c >> index 0fc905c230ad..d9c239933992 100644 >> --- a/tools/lib/traceevent/parse-filter.c >> +++ b/tools/lib/traceevent/parse-filter.c >> @@ -161,11 +161,13 @@ add_filter_type(struct event_filter *filter, int id) >> if (filter_type) >> return filter_type; >> >> - filter->event_filters = realloc(filter->event_filters, >> - sizeof(*filter->event_filters) * >> - (filter->filters + 1)); >> - if (!filter->event_filters) >> - die("Could not allocate filter"); >> + filter_type = realloc(filter->event_filters, >> + sizeof(*filter->event_filters) * >> + (filter->filters + 1)); >> + if (!filter_type) >> + return NULL; >> + >> + filter->event_filters = filter_type; >> >> for (i = 0; i < filter->filters; i++) { >> if (filter->event_filters[i].event_id > id) >> @@ -1164,6 +1166,12 @@ static int filter_event(struct event_filter *filter, >> } >> >> filter_type = add_filter_type(filter, event->id); >> + if (filter_type == NULL) { >> + show_error(error_str, "failed to add a new filter: %s", >> + filter_str ? filter_str : "true"); >> + return -1; > > so your key for using show_error in case of error is if it's > used already in the error path in the function.. right?
Right. In fact, it might look better if copy_filter_type() or pevent_filter_copy() pass error_str too. But the pevent_filter_ copy() looks like an user API so I was not sure I can change the signature. Also user can see its return value anyway and it's relatively simple function so might not care about the detail.. Thanks, Namhyung > >> + } >> + >> if (filter_type->filter) >> free_arg(filter_type->filter); >> filter_type->filter = arg; >> @@ -1395,6 +1403,9 @@ static int copy_filter_type(struct event_filter >> *filter, >> arg->boolean.value = 0; >> >> filter_type = add_filter_type(filter, event->id); >> + if (filter_type == NULL) >> + return -1; >> + >> filter_type->filter = arg; >> >> free(str); >> -- >> 1.7.11.7 >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
