From: David Goulet Date: Thu, 10 Jul 2014 17:04:24 +0000 (-0400) Subject: Fix: refactor function that fixes memory leak also X-Git-Tag: v2.6.0-rc1~155 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=137b99423df05ce6eb6982ba0a4f0d7b58d48513 Fix: refactor function that fixes memory leak also Signed-off-by: David Goulet --- diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index 29f8a10d4..d2f6799e1 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -743,6 +743,118 @@ end: return jul_filter; } +/* + * Generate the filter bytecode from a give filter expression string. Put the + * newly allocated parser context in ctxp and populate the lsm object with the + * expression len. + * + * Return 0 on success else a LTTNG_ERR_* code and ctxp is untouched. + */ +static int generate_filter(char *filter_expression, + struct lttcomm_session_msg *lsm, struct filter_parser_ctx **ctxp) +{ + int ret; + struct filter_parser_ctx *ctx = NULL; + FILE *fmem = NULL; + + assert(filter_expression); + assert(lsm); + assert(ctxp); + + /* + * Casting const to non-const, as the underlying function will use it in + * read-only mode. + */ + fmem = lttng_fmemopen((void *) filter_expression, + strlen(filter_expression), "r"); + if (!fmem) { + fprintf(stderr, "Error opening memory as stream\n"); + ret = -LTTNG_ERR_FILTER_NOMEM; + goto error; + } + ctx = filter_parser_ctx_alloc(fmem); + if (!ctx) { + fprintf(stderr, "Error allocating parser\n"); + ret = -LTTNG_ERR_FILTER_NOMEM; + goto filter_alloc_error; + } + ret = filter_parser_ctx_append_ast(ctx); + if (ret) { + fprintf(stderr, "Parse error\n"); + ret = -LTTNG_ERR_FILTER_INVAL; + goto parse_error; + } + ret = filter_visitor_set_parent(ctx); + if (ret) { + fprintf(stderr, "Set parent error\n"); + ret = -LTTNG_ERR_FILTER_INVAL; + goto parse_error; + } + if (print_xml) { + ret = filter_visitor_print_xml(ctx, stdout, 0); + if (ret) { + fflush(stdout); + fprintf(stderr, "XML print error\n"); + ret = -LTTNG_ERR_FILTER_INVAL; + goto parse_error; + } + } + + dbg_printf("Generating IR... "); + fflush(stdout); + ret = filter_visitor_ir_generate(ctx); + if (ret) { + fprintf(stderr, "Generate IR error\n"); + ret = -LTTNG_ERR_FILTER_INVAL; + goto parse_error; + } + dbg_printf("done\n"); + + dbg_printf("Validating IR... "); + fflush(stdout); + ret = filter_visitor_ir_check_binary_op_nesting(ctx); + if (ret) { + ret = -LTTNG_ERR_FILTER_INVAL; + goto parse_error; + } + dbg_printf("done\n"); + + dbg_printf("Generating bytecode... "); + fflush(stdout); + ret = filter_visitor_bytecode_generate(ctx); + if (ret) { + fprintf(stderr, "Generate bytecode error\n"); + ret = -LTTNG_ERR_FILTER_INVAL; + goto parse_error; + } + dbg_printf("done\n"); + dbg_printf("Size of bytecode generated: %u bytes.\n", + bytecode_get_len(&ctx->bytecode->b)); + + lsm->u.enable.bytecode_len = sizeof(ctx->bytecode->b) + + bytecode_get_len(&ctx->bytecode->b); + lsm->u.enable.expression_len = strlen(filter_expression) + 1; + + /* No need to keep the memory stream. */ + if (fclose(fmem) != 0) { + perror("fclose"); + } + + *ctxp = ctx; + return 0; + +parse_error: + filter_bytecode_free(ctx); + filter_ir_free(ctx); + filter_parser_ctx_free(ctx); +filter_alloc_error: + if (fclose(fmem) != 0) { + perror("fclose"); + } +error: + return ret; +} + /* * Enable event(s) for a channel, possibly with exclusions and a filter. * If no event name is specified, all events are enabled. @@ -759,18 +871,18 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, struct lttcomm_session_msg lsm; char *varlen_data; int ret = 0; + unsigned int free_filter_expression = 0; struct filter_parser_ctx *ctx = NULL; - FILE *fmem = NULL; /* * Cast as non-const since we may replace the filter expression * by a dynamically allocated string. Otherwise, the original * string is not modified. */ char *filter_expression = (char *) original_filter_expression; - int free_filter_expression = 0; if (handle == NULL || ev == NULL) { - return -LTTNG_ERR_INVALID; + ret = -LTTNG_ERR_INVALID; + goto error; } /* Empty filter string will always be rejected by the parser @@ -778,7 +890,8 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, * lttng_fmemopen error for 0-byte allocation. */ if (filter_expression && filter_expression[0] == '\0') { - return -LTTNG_ERR_INVALID; + ret = -LTTNG_ERR_INVALID; + goto error; } memset(&lsm, 0, sizeof(lsm)); @@ -828,9 +941,11 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, /* Setup JUL filter if needed. */ jul_filter = set_jul_filter(filter_expression, ev); - if (!jul_filter && !filter_expression) { - /* No JUL and no filter, just skip everything below. */ - goto ask_sessiond; + if (!jul_filter) { + if (!filter_expression) { + /* No JUL and no filter, just skip everything below. */ + goto ask_sessiond; + } } else { /* * With a JUL filter, the original filter has been added to it @@ -841,92 +956,24 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, } } - /* - * casting const to non-const, as the underlying function will - * use it in read-only mode. - */ - fmem = lttng_fmemopen((void *) filter_expression, - strlen(filter_expression), "r"); - if (!fmem) { - fprintf(stderr, "Error opening memory as stream\n"); - return -LTTNG_ERR_FILTER_NOMEM; - } - ctx = filter_parser_ctx_alloc(fmem); - if (!ctx) { - fprintf(stderr, "Error allocating parser\n"); - ret = -LTTNG_ERR_FILTER_NOMEM; - goto filter_alloc_error; - } - ret = filter_parser_ctx_append_ast(ctx); - if (ret) { - fprintf(stderr, "Parse error\n"); - ret = -LTTNG_ERR_FILTER_INVAL; - goto parse_error; - } - ret = filter_visitor_set_parent(ctx); - if (ret) { - fprintf(stderr, "Set parent error\n"); - ret = -LTTNG_ERR_FILTER_INVAL; - goto parse_error; - } - if (print_xml) { - ret = filter_visitor_print_xml(ctx, stdout, 0); - if (ret) { - fflush(stdout); - fprintf(stderr, "XML print error\n"); - ret = -LTTNG_ERR_FILTER_INVAL; - goto parse_error; - } - } - - dbg_printf("Generating IR... "); - fflush(stdout); - ret = filter_visitor_ir_generate(ctx); - if (ret) { - fprintf(stderr, "Generate IR error\n"); - ret = -LTTNG_ERR_FILTER_INVAL; - goto parse_error; - } - dbg_printf("done\n"); - - dbg_printf("Validating IR... "); - fflush(stdout); - ret = filter_visitor_ir_check_binary_op_nesting(ctx); + ret = generate_filter(filter_expression, &lsm, &ctx); if (ret) { - ret = -LTTNG_ERR_FILTER_INVAL; - goto parse_error; - } - dbg_printf("done\n"); - - dbg_printf("Generating bytecode... "); - fflush(stdout); - ret = filter_visitor_bytecode_generate(ctx); - if (ret) { - fprintf(stderr, "Generate bytecode error\n"); - ret = -LTTNG_ERR_FILTER_INVAL; - goto parse_error; + goto filter_error; } - dbg_printf("done\n"); - dbg_printf("Size of bytecode generated: %u bytes.\n", - bytecode_get_len(&ctx->bytecode->b)); - - lsm.u.enable.bytecode_len = sizeof(ctx->bytecode->b) - + bytecode_get_len(&ctx->bytecode->b); - lsm.u.enable.expression_len = strlen(filter_expression) + 1; } varlen_data = zmalloc(lsm.u.enable.bytecode_len - + lsm.u.enable.expression_len - + LTTNG_SYMBOL_NAME_LEN * exclusion_count); + + lsm.u.enable.expression_len + + LTTNG_SYMBOL_NAME_LEN * exclusion_count); if (!varlen_data) { ret = -LTTNG_ERR_EXCLUSION_NOMEM; - goto varlen_alloc_error; + goto filter_error; } + /* Put exclusion names first in the data */ while (exclusion_count--) { strncpy(varlen_data + LTTNG_SYMBOL_NAME_LEN * exclusion_count, - *(exclusion_list + exclusion_count), - LTTNG_SYMBOL_NAME_LEN); + *(exclusion_list + exclusion_count), LTTNG_SYMBOL_NAME_LEN); } /* Add filter expression next */ if (lsm.u.enable.expression_len != 0) { @@ -936,7 +983,7 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, lsm.u.enable.expression_len); } /* Add filter bytecode next */ - if (lsm.u.enable.bytecode_len != 0) { + if (ctx && lsm.u.enable.bytecode_len != 0) { memcpy(varlen_data + LTTNG_SYMBOL_NAME_LEN * lsm.u.enable.exclusion_count + lsm.u.enable.expression_len, @@ -946,18 +993,14 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, ret = lttng_ctl_ask_sessiond_varlen(&lsm, varlen_data, (LTTNG_SYMBOL_NAME_LEN * lsm.u.enable.exclusion_count) + - lsm.u.enable.bytecode_len + lsm.u.enable.expression_len, - NULL); + lsm.u.enable.bytecode_len + lsm.u.enable.expression_len, NULL); free(varlen_data); -varlen_alloc_error: +filter_error: if (filter_expression) { filter_bytecode_free(ctx); filter_ir_free(ctx); filter_parser_ctx_free(ctx); - if (fclose(fmem) != 0) { - perror("fclose"); - } if (free_filter_expression) { /* * The filter expression has been replaced and must be @@ -967,16 +1010,11 @@ varlen_alloc_error: free(filter_expression); } } - return ret; - -parse_error: - filter_bytecode_free(ctx); - filter_ir_free(ctx); - filter_parser_ctx_free(ctx); -filter_alloc_error: - if (fclose(fmem) != 0) { - perror("fclose"); - } +error: + /* + * Return directly to the caller and don't ask the sessiond since something + * went wrong in the parsing of data above. + */ return ret; ask_sessiond: