Code Review / openocd.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | review | tree
raw | patch | inline | side by side (parent: ff94e02)
gdb_server: Further cleanup of target desc functions 45/1645/2
authorAndreas Fritiofson <andreas.fritiofson@gmail.com>
Sun, 22 Sep 2013 11:01:00 +0000 (13:01 +0200)
committerAndreas Fritiofson <andreas.fritiofson@gmail.com>
Thu, 3 Oct 2013 21:10:24 +0000 (21:10 +0000)
Fix use of uninitialized pointer passed to xml_printf, caught by valgrind.

Make sure a failed gdb_generate_target_description frees all allocated
memory and avoids touching its out argument.

Plug memory leak and check allocation in handle_gdb_save_tdesc_command.

Change-Id: I30e20f6760a6215b1b4496304acdf47347eed829
Signed-off-by: Andreas Fritiofson <andreas.fritiofson@gmail.com>
Reviewed-on: http://openocd.zylin.com/1645
Tested-by: jenkins
Reviewed-by: Spencer Oliver <spen@spen-soft.co.uk>
Reviewed-by: Franck Jullien <franck.jullien@gmail.com>
src/server/gdb_server.c patch | blob | history

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 97cf26f2f25cc0e55c265991ff9be7113615da5f..660e05ef949c3be64c0e38955bfd49ffa921ca22 100644 (file)
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -2014,19 +2014,15 @@ static int get_reg_features_list(struct target *target, char **feature_list[], i
        return ERROR_OK;
 }
 
        return ERROR_OK;
 }
 
-static int gdb_generate_target_description(struct target *target, char **tdesc)
+static int gdb_generate_target_description(struct target *target, char **tdesc_out)
 {
        int retval = ERROR_OK;
        struct reg **reg_list;
        int reg_list_size;
 {
        int retval = ERROR_OK;
        struct reg **reg_list;
        int reg_list_size;
+       char *tdesc = NULL;
        int pos = 0;
        int size = 0;
 
        int pos = 0;
        int size = 0;
 
-       xml_printf(&retval, tdesc, &pos, &size,
-                       "<?xml version=\"1.0\"?>\n"
-                       "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n"
-                       "<target version=\"1.0\">\n");
-
        retval = target_get_gdb_reg_list(target, &reg_list,
                        &reg_list_size, REG_CLASS_ALL);
 
        retval = target_get_gdb_reg_list(target, &reg_list,
                        &reg_list_size, REG_CLASS_ALL);
 
@@ -2035,25 +2031,33 @@ static int gdb_generate_target_description(struct target *target, char **tdesc)
                return ERROR_FAIL;
        }
 
                return ERROR_FAIL;
        }
 
-       if (reg_list_size <= 0)
+       if (reg_list_size <= 0) {
+               free(reg_list);
                return ERROR_FAIL;
                return ERROR_FAIL;
+       }
 
        char **features = NULL;
        /* Get a list of available target registers features */
        retval = get_reg_features_list(target, &features, NULL, reg_list, reg_list_size);
        if (retval != ERROR_OK) {
                LOG_ERROR("Can't get the registers feature list");
 
        char **features = NULL;
        /* Get a list of available target registers features */
        retval = get_reg_features_list(target, &features, NULL, reg_list, reg_list_size);
        if (retval != ERROR_OK) {
                LOG_ERROR("Can't get the registers feature list");
+               free(reg_list);
                return ERROR_FAIL;
        }
 
        /* If we found some features associated with registers, create sections */
        int current_feature = 0;
 
                return ERROR_FAIL;
        }
 
        /* If we found some features associated with registers, create sections */
        int current_feature = 0;
 
+       xml_printf(&retval, &tdesc, &pos, &size,
+                       "<?xml version=\"1.0\"?>\n"
+                       "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n"
+                       "<target version=\"1.0\">\n");
+
        /* generate target description according to register list */
        if (features != NULL) {
                while (features[current_feature]) {
 
        /* generate target description according to register list */
        if (features != NULL) {
                while (features[current_feature]) {
 
-                       xml_printf(&retval, tdesc, &pos, &size,
+                       xml_printf(&retval, &tdesc, &pos, &size,
                                        "<feature name=\"%s\">\n",
                                        features[current_feature]);
 
                                        "<feature name=\"%s\">\n",
                                        features[current_feature]);
 
@@ -2070,7 +2074,7 @@ static int gdb_generate_target_description(struct target *target, char **tdesc)
                                if (reg_list[i]->reg_data_type != NULL) {
                                        if (reg_list[i]->reg_data_type->type == REG_TYPE_ARCH_DEFINED) {
                                                /* generate <type... first, if there are architecture-defined types. */
                                if (reg_list[i]->reg_data_type != NULL) {
                                        if (reg_list[i]->reg_data_type->type == REG_TYPE_ARCH_DEFINED) {
                                                /* generate <type... first, if there are architecture-defined types. */
-                                               gdb_generate_reg_type_description(target, tdesc, &pos, &size,
+                                               gdb_generate_reg_type_description(target, &tdesc, &pos, &size,
                                                                reg_list[i]->reg_data_type);
 
                                                type_str = reg_list[i]->reg_data_type->id;
                                                                reg_list[i]->reg_data_type);
 
                                                type_str = reg_list[i]->reg_data_type->id;
@@ -2084,47 +2088,49 @@ static int gdb_generate_target_description(struct target *target, char **tdesc)
                                        type_str = "int";
                                }
 
                                        type_str = "int";
                                }
 
-                               xml_printf(&retval, tdesc, &pos, &size,
+                               xml_printf(&retval, &tdesc, &pos, &size,
                                                "<reg name=\"%s\"", reg_list[i]->name);
                                                "<reg name=\"%s\"", reg_list[i]->name);
-                               xml_printf(&retval, tdesc, &pos, &size,
+                               xml_printf(&retval, &tdesc, &pos, &size,
                                                " bitsize=\"%d\"", reg_list[i]->size);
                                                " bitsize=\"%d\"", reg_list[i]->size);
-                               xml_printf(&retval, tdesc, &pos, &size,
+                               xml_printf(&retval, &tdesc, &pos, &size,
                                                " regnum=\"%d\"", reg_list[i]->number);
                                if (reg_list[i]->caller_save)
                                                " regnum=\"%d\"", reg_list[i]->number);
                                if (reg_list[i]->caller_save)
-                                       xml_printf(&retval, tdesc, &pos, &size,
+                                       xml_printf(&retval, &tdesc, &pos, &size,
                                                        " save-restore=\"yes\"");
                                else
                                                        " save-restore=\"yes\"");
                                else
-                                       xml_printf(&retval, tdesc, &pos, &size,
+                                       xml_printf(&retval, &tdesc, &pos, &size,
                                                        " save-restore=\"no\"");
 
                                                        " save-restore=\"no\"");
 
-                               xml_printf(&retval, tdesc, &pos, &size,
+                               xml_printf(&retval, &tdesc, &pos, &size,
                                                " type=\"%s\"", type_str);
 
                                if (reg_list[i]->group != NULL)
                                                " type=\"%s\"", type_str);
 
                                if (reg_list[i]->group != NULL)
-                                       xml_printf(&retval, tdesc, &pos, &size,
+                                       xml_printf(&retval, &tdesc, &pos, &size,
                                                        " group=\"%s\"", reg_list[i]->group);
 
                                                        " group=\"%s\"", reg_list[i]->group);
 
-                               xml_printf(&retval, tdesc, &pos, &size,
+                               xml_printf(&retval, &tdesc, &pos, &size,
                                                "/>\n");
                        }
 
                                                "/>\n");
                        }
 
-                       xml_printf(&retval, tdesc, &pos, &size,
+                       xml_printf(&retval, &tdesc, &pos, &size,
                                        "</feature>\n");
 
                        current_feature++;
                }
        }
 
                                        "</feature>\n");
 
                        current_feature++;
                }
        }
 
-       xml_printf(&retval, tdesc, &pos, &size,
+       xml_printf(&retval, &tdesc, &pos, &size,
                        "</target>\n");
 
                        "</target>\n");
 
-       if (reg_list != NULL)
-               free(reg_list);
+       free(reg_list);
+       free(features);
 
 
-       if (features != NULL)
-               free(features);
+       if (retval == ERROR_OK)
+               *tdesc_out = tdesc;
+       else
+               free(tdesc);
 
 
-       return ERROR_OK;
+       return retval;
 }
 
 static int gdb_get_target_description_chunk(struct target *target, struct target_desc_format *target_desc,
 }
 
 static int gdb_get_target_description_chunk(struct target *target, struct target_desc_format *target_desc,
@@ -3012,7 +3018,6 @@ COMMAND_HANDLER(handle_gdb_save_tdesc_command)
        char *tdesc;
        uint32_t tdesc_length;
        struct target *target = get_current_target(CMD_CTX);
        char *tdesc;
        uint32_t tdesc_length;
        struct target *target = get_current_target(CMD_CTX);
-       char *tdesc_filename;
 
        int retval = gdb_generate_target_description(target, &tdesc);
        if (retval != ERROR_OK) {
 
        int retval = gdb_generate_target_description(target, &tdesc);
        if (retval != ERROR_OK) {
@@ -3022,37 +3027,34 @@ COMMAND_HANDLER(handle_gdb_save_tdesc_command)
 
        tdesc_length = strlen(tdesc);
 
 
        tdesc_length = strlen(tdesc);
 
-       if (tdesc_length == 0) {
-               LOG_ERROR("Unable to Generate Target Description");
-               return ERROR_FAIL;
-       }
-
        struct fileio fileio;
        size_t size_written;
 
        struct fileio fileio;
        size_t size_written;
 
-       tdesc_filename = malloc(strlen(target_type_name(target)) + 5);
-       sprintf(tdesc_filename, "%s.xml", target_type_name(target));
+       char *tdesc_filename = alloc_printf("%s.xml", target_type_name(target));
+       if (tdesc_filename == NULL) {
+               retval = ERROR_FAIL;
+               goto out;
+       }
 
        retval = fileio_open(&fileio, tdesc_filename, FILEIO_WRITE, FILEIO_TEXT);
 
        if (retval != ERROR_OK) {
 
        retval = fileio_open(&fileio, tdesc_filename, FILEIO_WRITE, FILEIO_TEXT);
 
        if (retval != ERROR_OK) {
-               LOG_WARNING("Can't open %s for writing", tdesc_filename);
-               free(tdesc_filename);
-               return ERROR_FAIL;
+               LOG_ERROR("Can't open %s for writing", tdesc_filename);
+               goto out;
        }
 
        retval = fileio_write(&fileio, tdesc_length, tdesc, &size_written);
 
        fileio_close(&fileio);
        }
 
        retval = fileio_write(&fileio, tdesc_length, tdesc, &size_written);
 
        fileio_close(&fileio);
+
+       if (retval != ERROR_OK)
+               LOG_ERROR("Error while writing the tdesc file");
+
+out:
        free(tdesc_filename);
        free(tdesc);
 
        free(tdesc_filename);
        free(tdesc);
 
-       if (retval != ERROR_OK) {
-               LOG_WARNING("Error while writing the tdesc file");
-               return ERROR_FAIL;
-       }
-
-       return ERROR_OK;
+       return retval;
 }
 
 static const struct command_registration gdb_command_handlers[] = {
 }
 
 static const struct command_registration gdb_command_handlers[] = {
Open On-Chip Debugger

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)