This is an automated email from Gerrit.

Marc Schink ([email protected]) just uploaded a new patch set to 
Gerrit, which you can find at http://openocd.zylin.com/2959

-- gerrit

commit fc5a31a3919c16e70dba414bf9b05c9eb4a508f5
Author: Marc Schink <[email protected]>
Date:   Wed Aug 12 17:42:22 2015 +0200

    Fix Jim interpreter memory leak.
    
    Change-Id: I71d7d97e7dc315c42fc43b65cb5fcecd7bdfb581
    Signed-off-by: Marc Schink <[email protected]>

diff --git a/src/flash/nor/stellaris.c b/src/flash/nor/stellaris.c
index 451f19b..031fa8c 100644
--- a/src/flash/nor/stellaris.c
+++ b/src/flash/nor/stellaris.c
@@ -1358,6 +1358,7 @@ COMMAND_HANDLER(stellaris_handle_recover_command)
 {
        struct flash_bank *bank;
        int retval;
+       const char *tmp;
 
        if (CMD_ARGC != 0)
                return ERROR_COMMAND_SYNTAX_ERROR;
@@ -1372,8 +1373,11 @@ COMMAND_HANDLER(stellaris_handle_recover_command)
         * cycle to recover.
         */
 
-       Jim_Eval_Named(CMD_CTX->interp, "catch { hla_command \"debug unlock\" 
}", 0, 0);
-       if (!strcmp(Jim_GetString(Jim_GetResult(CMD_CTX->interp), NULL), "0")) {
+       Jim_Eval_Named(CMD_CTX->interp->interp,
+               "catch { hla_command \"debug unlock\" }", 0, 0);
+       tmp = Jim_GetString(Jim_GetResult(CMD_CTX->interp->interp), NULL);
+
+       if (!strcmp(tmp, "0")) {
                retval = ERROR_OK;
                goto user_action;
        }
diff --git a/src/helper/command.c b/src/helper/command.c
index a0aa9e8..171d002 100644
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -358,7 +358,7 @@ static int command_unknown(Jim_Interp *interp, int argc, 
Jim_Obj *const *argv);
 static int register_command_handler(struct command_context *cmd_ctx,
        struct command *c)
 {
-       Jim_Interp *interp = cmd_ctx->interp;
+       Jim_Interp *interp = cmd_ctx->interp->interp;
        char *ocd_name = alloc_printf("ocd_%s", c->name);
        if (NULL == ocd_name)
                return JIM_ERR;
@@ -408,7 +408,7 @@ struct command *register_command(struct command_context 
*context,
 
        int retval = ERROR_OK;
        if (NULL != cr->jim_handler && NULL == parent) {
-               retval = Jim_CreateCommand(context->interp, cr->name,
+               retval = Jim_CreateCommand(context->interp->interp, cr->name,
                                cr->jim_handler, cr->jim_handler_data, NULL);
        } else if (NULL != cr->handler || NULL != parent)
                retval = register_command_handler(context, command_root(c));
@@ -645,7 +645,7 @@ int command_run_line(struct command_context *context, char 
*line)
         * happen when the Jim Tcl interpreter is provided by eCos for
         * instance.
         */
-       Jim_Interp *interp = context->interp;
+       Jim_Interp *interp = context->interp->interp;
        Jim_DeleteAssocData(interp, "context");
        retcode = Jim_SetAssocData(interp, "context", NULL, context);
        if (retcode == JIM_OK) {
@@ -722,8 +722,14 @@ void command_set_output_handler(struct command_context 
*context,
 
 struct command_context *copy_command_context(struct command_context *context)
 {
-       struct command_context *copy_context = malloc(sizeof(struct 
command_context));
+       struct command_context *copy_context;
 
+       copy_context = malloc(sizeof(struct command_context));
+
+       if (!copy_context)
+               return NULL;
+
+       context->interp->refcnt++;
        *copy_context = *context;
 
        return copy_context;
@@ -731,9 +737,18 @@ struct command_context *copy_command_context(struct 
command_context *context)
 
 void command_done(struct command_context *cmd_ctx)
 {
-       if (NULL == cmd_ctx)
+       if (cmd_ctx == NULL)
                return;
 
+       cmd_ctx->interp->refcnt--;
+
+       if (!cmd_ctx->interp->refcnt) {
+               if (cmd_ctx->interp->free)
+                       Jim_FreeInterp(cmd_ctx->interp->interp);
+
+               free(cmd_ctx->interp);
+       }
+
        free(cmd_ctx);
 }
 
@@ -1271,14 +1286,28 @@ static const struct command_registration 
command_builtin_handlers[] = {
 
 struct command_context *command_init(const char *startup_tcl, Jim_Interp 
*interp)
 {
-       struct command_context *context = malloc(sizeof(struct 
command_context));
+       struct command_context *context;
        const char *HostOs;
 
+       context = malloc(sizeof(struct command_context));
+
+       if (!context) {
+               LOG_ERROR("Failed to allocate command context.");
+               return NULL;
+       }
+
        context->mode = COMMAND_EXEC;
        context->commands = NULL;
        context->current_target = 0;
        context->output_handler = NULL;
        context->output_handler_priv = NULL;
+       context->interp = malloc(sizeof(struct command_interpreter));
+
+       if (!context->interp) {
+               LOG_ERROR("Failed to allocate command interpreter.");
+               free(context);
+               return NULL;
+       }
 
        /* Create a jim interpreter if we were not handed one */
        if (interp == NULL) {
@@ -1287,9 +1316,13 @@ struct command_context *command_init(const char 
*startup_tcl, Jim_Interp *interp
                /* Add all the Jim core commands */
                Jim_RegisterCoreCommands(interp);
                Jim_InitStaticExtensions(interp);
+               context->interp->free = true;
+       } else {
+               context->interp->free = false;
        }
 
-       context->interp = interp;
+       context->interp->interp = interp;
+       context->interp->refcnt = 1;
 
        /* Stick to lowercase for HostOS strings. */
 #if defined(_MSC_VER)
@@ -1357,7 +1390,7 @@ void process_jim_events(struct command_context *cmd_ctx)
                return;
 
        recursion++;
-       Jim_ProcessEvents(cmd_ctx->interp, JIM_ALL_EVENTS | JIM_DONT_WAIT);
+       Jim_ProcessEvents(cmd_ctx->interp->interp, JIM_ALL_EVENTS | 
JIM_DONT_WAIT);
        recursion--;
 }
 
diff --git a/src/helper/command.h b/src/helper/command.h
index 0eda5b5..0af0fe7 100644
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -41,6 +41,22 @@ enum command_mode {
        COMMAND_ANY,
 };
 
+/**
+ * Helper struct to share a common Jim interpreter among multiple command
+ * contexts.
+ */
+struct command_interpreter {
+       /** Jim interpreter. */
+       Jim_Interp *interp;
+       /** Number of references held on this interpreter. */
+       unsigned int refcnt;
+       /**
+        * Determines whether the Jim interpreter should be free'd when the last
+        * reference is released.
+        */
+       bool free;
+};
+
 struct command_context;
 
 /** The type signature for command context's output handler. */
@@ -48,7 +64,7 @@ typedef int (*command_output_handler_t)(struct 
command_context *context,
                const char *line);
 
 struct command_context {
-       Jim_Interp *interp;
+       struct command_interpreter *interp;
        enum command_mode mode;
        struct command *commands;
        int current_target;
diff --git a/src/jtag/core.c b/src/jtag/core.c
index 74c2731..0992573 100644
--- a/src/jtag/core.c
+++ b/src/jtag/core.c
@@ -1580,7 +1580,10 @@ int jtag_init(struct command_context *cmd_ctx)
        if (retval != ERROR_OK)
                return retval;
 
-       if (Jim_Eval_Named(cmd_ctx->interp, "jtag_init", __FILE__, __LINE__) != 
JIM_OK)
+       retval = Jim_Eval_Named(cmd_ctx->interp->interp, "jtag_init", __FILE__,
+               __LINE__);
+
+       if (retval != JIM_OK)
                return ERROR_FAIL;
 
        return ERROR_OK;
diff --git a/src/server/server.c b/src/server/server.c
index 7e90d89..63244f1 100644
--- a/src/server/server.c
+++ b/src/server/server.c
@@ -54,7 +54,8 @@ static int last_signal;
 /* set the polling period to 100ms */
 static int polling_period = 100;
 
-static int add_connection(struct service *service, struct command_context 
*cmd_ctx)
+static int add_connection(struct service *service,
+               struct command_context *cmd_ctx)
 {
        socklen_t address_size;
        struct connection *c, **p;
@@ -62,10 +63,23 @@ static int add_connection(struct service *service, struct 
command_context *cmd_c
        int flag = 1;
 
        c = malloc(sizeof(struct connection));
+
+       if (!c) {
+               LOG_ERROR("Failed to allocate connection.");
+               return ERROR_FAIL;
+       }
+
+       c->cmd_ctx = copy_command_context(cmd_ctx);
+
+       if (!c->cmd_ctx) {
+               LOG_ERROR("Failed to copy command context.");
+               free(c);
+               return ERROR_FAIL;
+       }
+
        c->fd = -1;
        c->fd_out = -1;
        memset(&c->sin, 0, sizeof(c->sin));
-       c->cmd_ctx = copy_command_context(cmd_ctx);
        c->service = service;
        c->input_pending = 0;
        c->priv = NULL;
diff --git a/src/server/tcl_server.c b/src/server/tcl_server.c
index 409567c..56eecd9 100644
--- a/src/server/tcl_server.c
+++ b/src/server/tcl_server.c
@@ -167,7 +167,7 @@ static int tcl_new_connection(struct connection *connection)
 
 static int tcl_input(struct connection *connection)
 {
-       Jim_Interp *interp = (Jim_Interp *)connection->cmd_ctx->interp;
+       Jim_Interp *interp;
        int retval;
        int i;
        ssize_t rlen;
@@ -176,6 +176,8 @@ static int tcl_input(struct connection *connection)
        struct tcl_connection *tclc;
        unsigned char in[256];
 
+       interp = connection->cmd_ctx->interp->interp;
+
        rlen = connection_read(connection, &in, sizeof(in));
        if (rlen <= 0) {
                if (rlen < 0)
diff --git a/src/target/dsp563xx.c b/src/target/dsp563xx.c
index 1cb18cf..3e0fb43 100644
--- a/src/target/dsp563xx.c
+++ b/src/target/dsp563xx.c
@@ -1472,7 +1472,7 @@ static int dsp563xx_get_default_memory(void)
        if (!global_cmd_ctx)
                return MEM_P;
 
-       interp = global_cmd_ctx->interp;
+       interp = global_cmd_ctx->interp->interp;
 
        if (!interp)
                return MEM_P;
diff --git a/src/target/target.c b/src/target/target.c
index 19e5d65..5460517 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -640,13 +640,14 @@ static int target_process_reset(struct command_context 
*cmd_ctx, enum target_res
        jtag_poll_set_enabled(false);
 
        sprintf(buf, "ocd_process_reset %s", n->name);
-       retval = Jim_Eval(cmd_ctx->interp, buf);
+       retval = Jim_Eval(cmd_ctx->interp->interp, buf);
 
        jtag_poll_set_enabled(save_poll);
 
        if (retval != JIM_OK) {
-               Jim_MakeErrorMessage(cmd_ctx->interp);
-               command_print(NULL, "%s\n", 
Jim_GetString(Jim_GetResult(cmd_ctx->interp), NULL));
+               Jim_MakeErrorMessage(cmd_ctx->interp->interp);
+               command_print(NULL, "%s\n",
+                       Jim_GetString(Jim_GetResult(cmd_ctx->interp->interp), 
NULL));
                return ERROR_FAIL;
        }
 
@@ -1270,7 +1271,7 @@ static int target_init(struct command_context *cmd_ctx)
                return retval;
 
        retval = target_register_timer_callback(&handle_target,
-                       polling_interval, 1, cmd_ctx->interp);
+                       polling_interval, 1, cmd_ctx->interp->interp);
        if (ERROR_OK != retval)
                return retval;
 

-- 

------------------------------------------------------------------------------
_______________________________________________
OpenOCD-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openocd-devel

Reply via email to