Fix: refactor function that fixes memory leak also
authorDavid Goulet <dgoulet@efficios.com>
Thu, 10 Jul 2014 17:04:24 +0000 (13:04 -0400)
committerDavid Goulet <dgoulet@efficios.com>
Thu, 10 Jul 2014 17:06:36 +0000 (13:06 -0400)
Signed-off-by: David Goulet <dgoulet@efficios.com>
src/lib/lttng-ctl/lttng-ctl.c

index 29f8a10d4d65f7d2295a289793789e520f79ed08..d2f6799e11a070031805f09774e33b137c33d38c 100644 (file)
@@ -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:
This page took 0.029708 seconds and 4 git commands to generate.