From 4f885215816624df7bc19523ea2da94ca90772f3 Mon Sep 17 00:00:00 2001 From: Andreas Fritiofson Date: Sun, 22 Sep 2013 13:01:00 +0200 Subject: [PATCH] gdb_server: Further cleanup of target desc functions 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 Reviewed-on: http://openocd.zylin.com/1645 Tested-by: jenkins Reviewed-by: Spencer Oliver Reviewed-by: Franck Jullien --- src/server/gdb_server.c | 84 +++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index 97cf26f2f2..660e05ef94 100644 --- 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; } -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; + char *tdesc = NULL; int pos = 0; int size = 0; - xml_printf(&retval, tdesc, &pos, &size, - "\n" - "\n" - "\n"); - retval = target_get_gdb_reg_list(target, ®_list, ®_list_size, REG_CLASS_ALL); @@ -2035,25 +2031,33 @@ static int gdb_generate_target_description(struct target *target, char **tdesc) return ERROR_FAIL; } - if (reg_list_size <= 0) + if (reg_list_size <= 0) { + free(reg_list); 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"); + free(reg_list); return ERROR_FAIL; } /* If we found some features associated with registers, create sections */ int current_feature = 0; + xml_printf(&retval, &tdesc, &pos, &size, + "\n" + "\n" + "\n"); + /* 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, "\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 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"; } - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, "name); - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &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) - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, " save-restore=\"yes\""); else - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, " 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) - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, " group=\"%s\"", reg_list[i]->group); - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, "/>\n"); } - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, "\n"); current_feature++; } } - xml_printf(&retval, tdesc, &pos, &size, + xml_printf(&retval, &tdesc, &pos, &size, "\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, @@ -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_filename; 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); - if (tdesc_length == 0) { - LOG_ERROR("Unable to Generate Target Description"); - return ERROR_FAIL; - } - 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) { - 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); + + if (retval != ERROR_OK) + LOG_ERROR("Error while writing the tdesc file"); + +out: 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[] = { -- 2.30.2