Messages in this thread Patch in this message | | | Date | Mon, 9 Dec 2013 11:05:19 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH 04/14] tools lib traceevent: Get rid of malloc_or_die() allocate_arg() |
| |
On Mon, 9 Dec 2013 14:34:01 +0900 Namhyung Kim <namhyung@kernel.org> wrote:
> @@ -409,8 +408,10 @@ create_arg_op(enum filter_op_type btype) > struct filter_arg *arg; > > arg = allocate_arg(); > - arg->type = FILTER_ARG_OP; > - arg->op.type = btype; > + if (arg) { > + arg->type = FILTER_ARG_OP; > + arg->op.type = btype; > + } > > return arg; > } > @@ -421,8 +422,10 @@ create_arg_exp(enum filter_exp_type etype) > struct filter_arg *arg; > > arg = allocate_arg(); > - arg->type = FILTER_ARG_EXP; > - arg->op.type = etype; > + if (arg) { > + arg->type = FILTER_ARG_EXP; > + arg->op.type = etype; > + } > > return arg; > } > @@ -433,9 +436,11 @@ create_arg_cmp(enum filter_exp_type etype) > struct filter_arg *arg; > > arg = allocate_arg(); > - /* Use NUM and change if necessary */ > - arg->type = FILTER_ARG_NUM; > - arg->op.type = etype; > + if (arg) { > + /* Use NUM and change if necessary */ > + arg->type = FILTER_ARG_NUM; > + arg->op.type = etype; > + } > > return arg; > } > @@ -896,8 +901,10 @@ static struct filter_arg *collapse_tree(struct filter_arg *arg) > case FILTER_VAL_FALSE: > free_arg(arg); > arg = allocate_arg(); > - arg->type = FILTER_ARG_BOOLEAN; > - arg->boolean.value = ret == FILTER_VAL_TRUE; > + if (arg) { > + arg->type = FILTER_ARG_BOOLEAN; > + arg->boolean.value = ret == FILTER_VAL_TRUE; > + }
Just a nit, but I wonder if all these would look nicer if we just did:
arg = allocate_arg(); if (!arg) return NULL; [...]
Instead of doing the work within an if statement.
Also, I prefer if (!arg) over if (arg == NULL), but I'm not going to fight over that ;-)
-- Steve
> } > > return arg; > @@ -1044,6 +1051,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, > switch (op_type) { > case OP_BOOL: > arg = create_arg_op(btype); > + if (arg == NULL) > + goto fail_alloc; > if (current_op) > ret = add_left(arg, current_op); > else > @@ -1054,6 +1063,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, > > case OP_NOT: > arg = create_arg_op(btype); > + if (arg == NULL) > + goto fail_alloc; > if (current_op) > ret = add_right(current_op, arg, error_str); > if (ret < 0) > @@ -1073,6 +1084,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, > arg = create_arg_exp(etype); > else > arg = create_arg_cmp(ctype); > + if (arg == NULL) > + goto fail_alloc; > > if (current_op) > ret = add_right(current_op, arg, error_str); > @@ -1106,11 +1119,16 @@ process_filter(struct event_format *event, struct filter_arg **parg, > current_op = current_exp; > > current_op = collapse_tree(current_op); > + if (current_op == NULL) > + goto fail_alloc; > > *parg = current_op; > > return 0; > > + fail_alloc: > + show_error(error_str, "failed to allocate filter arg"); > + goto fail; > fail_print: > show_error(error_str, "Syntax error"); > fail: > @@ -1141,6 +1159,10 @@ process_event(struct event_format *event, const char *filter_str, > /* If parg is NULL, then make it into FALSE */ > if (!*parg) { > *parg = allocate_arg(); > + if (*parg == NULL) { > + show_error(error_str, "failed to allocate filter arg"); > + return -1; > + } > (*parg)->type = FILTER_ARG_BOOLEAN; > (*parg)->boolean.value = FILTER_FALSE; > } > @@ -1164,6 +1186,10 @@ static int filter_event(struct event_filter *filter, > } else { > /* just add a TRUE arg */ > arg = allocate_arg(); > + if (arg == NULL) { > + show_error(error_str, "failed to allocate filter arg"); > + return -1; > + } > arg->type = FILTER_ARG_BOOLEAN; > arg->boolean.value = FILTER_TRUE; > } > @@ -1399,6 +1425,9 @@ static int copy_filter_type(struct event_filter *filter, > if (strcmp(str, "TRUE") == 0 || strcmp(str, "FALSE") == 0) { > /* Add trivial event */ > arg = allocate_arg(); > + if (arg == NULL) > + return -1; > + > arg->type = FILTER_ARG_BOOLEAN; > if (strcmp(str, "TRUE") == 0) > arg->boolean.value = 1;
| |