From: oharboe Date: Mon, 10 Mar 2008 12:11:07 +0000 (+0000) Subject: - Fixed various error handling when looking for memory leaks X-Git-Url: https://git.gag.com/?a=commitdiff_plain;h=0424155dfc29ab8e3b5e4b7f36f9164df7b5b740;p=fw%2Fopenocd - Fixed various error handling when looking for memory leaks - Fixed memory leak in gdb_server.c - pushed "Error:" statements up into fn's that know something about what went wrong - load_image now fails if target_write_memory() fails - only issue an asynchronous halt() upon connect of GDB. Synchronous halt/reset doesn't really work as what's required to initialize the target might involve a special monitor sequence for the target in question - syntax error handling improved(fewer exit()'s) git-svn-id: svn://svn.berlios.de/openocd/trunk@482 b42882b7-edfa-0310-969c-e2dbd0fdcd60 --- diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index f01d638fd..ce45f2980 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -678,25 +678,23 @@ int gdb_new_connection(connection_t *connection) /* register callback to be informed about target events */ target_register_event_callback(gdb_target_callback_event_handler, connection); - /* a gdb session just attached, try to put the target in halt mode - * or alterantively try to issue a reset. - * - * GDB connection will fail if e.g. register read packets fail, - * otherwise resetting/halting the target could have been left to GDB init - * scripts + /* a gdb session just attached, try to put the target in halt mode. * * DANGER!!!! - * We need a synchronous halt, lest connect will fail. - * Also there is no guarantee that poll() will be invoked - * between here and serving the first packet, so the halt() - * statement above is *NOT* sufficient + * + * If the halt fails(e.g. target needs a reset, JTAG communication not + * working, etc.), then the GDB connect will succeed as + * the get_gdb_reg_list() will lie and return a register list with + * dummy values. + * + * This allows GDB monitor commands to be run from a GDB init script to + * initialize the target + * + * Also, since the halt() is asynchronous target connect will be + * instantaneous and thus avoiding annoying timeout problems during + * connect. */ - if ((retval = gdb_service->target->type->halt(gdb_service->target)) != ERROR_OK) - { - ERROR("error(%d) when trying to halt target, falling back to \"reset\"", retval); - command_run_line(connection->cmd_ctx, "reset"); - } - command_run_line(connection->cmd_ctx, "halt"); + gdb_service->target->type->halt(gdb_service->target); /* remove the initial ACK from the incoming buffer */ if ((retval = gdb_get_char(connection, &initial_ack)) != ERROR_OK) @@ -1182,13 +1180,12 @@ int gdb_write_memory_packet(connection_t *connection, target_t *target, char *pa } else { - if ((retval = gdb_error(connection, retval)) != ERROR_OK) - return retval; + retval = gdb_error(connection, retval); } free(buffer); - return ERROR_OK; + return retval; } int gdb_write_memory_binary_packet(connection_t *connection, target_t *target, char *packet, int packet_size) diff --git a/src/target/target.c b/src/target/target.c index dc69e9944..51aef65e9 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -749,7 +749,7 @@ int target_register_commands(struct command_context_s *cmd_ctx) register_command(cmd_ctx, NULL, "targets", handle_targets_command, COMMAND_EXEC, NULL); register_command(cmd_ctx, NULL, "daemon_startup", handle_daemon_startup_command, COMMAND_CONFIG, NULL); register_command(cmd_ctx, NULL, "target_script", handle_target_script_command, COMMAND_CONFIG, NULL); - register_command(cmd_ctx, NULL, "run_and_halt_time", handle_run_and_halt_time_command, COMMAND_CONFIG, NULL); + register_command(cmd_ctx, NULL, "run_and_halt_time", handle_run_and_halt_time_command, COMMAND_CONFIG, " "); register_command(cmd_ctx, NULL, "working_area", handle_working_area_command, COMMAND_ANY, "working_area
<'backup'|'nobackup'> [virtual address]"); register_command(cmd_ctx, NULL, "virt2phys", handle_virt2phys_command, COMMAND_ANY, "virt2phys "); @@ -1110,8 +1110,7 @@ int handle_target_command(struct command_context_s *cmd_ctx, char *cmd, char **a if (argc < 3) { - ERROR("target command requires at least three arguments: "); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } /* search for the specified target */ @@ -1148,7 +1147,7 @@ int handle_target_command(struct command_context_s *cmd_ctx, char *cmd, char **a else { ERROR("endianness must be either 'little' or 'big', not '%s'", args[1]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } /* what to do on a target reset */ @@ -1165,7 +1164,7 @@ int handle_target_command(struct command_context_s *cmd_ctx, char *cmd, char **a else { ERROR("unknown target startup mode %s", args[2]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } (*last_target_p)->run_and_halt_time = 1000; /* default 1s */ @@ -1212,7 +1211,7 @@ int handle_target_command(struct command_context_s *cmd_ctx, char *cmd, char **a if (!found) { ERROR("target '%s' not found", args[0]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } return ERROR_OK; @@ -1226,15 +1225,14 @@ int handle_target_script_command(struct command_context_s *cmd_ctx, char *cmd, c if (argc < 3) { ERROR("incomplete target_script command"); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } target = get_target_by_num(strtoul(args[0], NULL, 0)); if (!target) { - ERROR("target number '%s' not defined", args[0]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } if (strcmp(args[1], "reset") == 0) @@ -1264,7 +1262,7 @@ int handle_target_script_command(struct command_context_s *cmd_ctx, char *cmd, c else { ERROR("unknown event type: '%s", args[1]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } return ERROR_OK; @@ -1276,16 +1274,14 @@ int handle_run_and_halt_time_command(struct command_context_s *cmd_ctx, char *cm if (argc < 2) { - ERROR("incomplete run_and_halt_time command"); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } target = get_target_by_num(strtoul(args[0], NULL, 0)); if (!target) { - ERROR("target number '%s' not defined", args[0]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } target->run_and_halt_time = strtoul(args[1], NULL, 0); @@ -1306,8 +1302,7 @@ int handle_working_area_command(struct command_context_s *cmd_ctx, char *cmd, ch if (!target) { - ERROR("target number '%s' not defined", args[0]); - exit(-1); + return ERROR_COMMAND_SYNTAX_ERROR; } target_free_all_working_areas(target); @@ -1898,6 +1893,7 @@ int handle_load_image_command(struct command_context_s *cmd_ctx, char *cmd, char } image_size = 0x0; + retval = ERROR_OK; for (i = 0; i < image.num_sections; i++) { buffer = malloc(image.sections[i].size); @@ -1909,13 +1905,14 @@ int handle_load_image_command(struct command_context_s *cmd_ctx, char *cmd, char if ((retval = image_read_section(&image, i, 0x0, image.sections[i].size, buffer, &buf_cnt)) != ERROR_OK) { - ERROR("image_read_section failed with error code: %i", retval); - command_print(cmd_ctx, "image reading failed, download aborted"); free(buffer); - image_close(&image); - return ERROR_OK; + break; + } + if ((retval = target_write_buffer(target, image.sections[i].base_address, buf_cnt, buffer)) != ERROR_OK) + { + free(buffer); + break; } - target_write_buffer(target, image.sections[i].base_address, buf_cnt, buffer); image_size += buf_cnt; command_print(cmd_ctx, "%u byte written at address 0x%8.8x", buf_cnt, image.sections[i].base_address); @@ -1923,12 +1920,15 @@ int handle_load_image_command(struct command_context_s *cmd_ctx, char *cmd, char } duration_stop_measure(&duration, &duration_text); - command_print(cmd_ctx, "downloaded %u byte in %s", image_size, duration_text); + if (retval==ERROR_OK) + { + command_print(cmd_ctx, "downloaded %u byte in %s", image_size, duration_text); + } free(duration_text); image_close(&image); - return ERROR_OK; + return retval; } @@ -1939,7 +1939,7 @@ int handle_dump_image_command(struct command_context_s *cmd_ctx, char *cmd, char u32 address; u32 size; u8 buffer[560]; - int retval; + int retval=ERROR_OK; duration_t duration; char *duration_text; @@ -1976,11 +1976,14 @@ int handle_dump_image_command(struct command_context_s *cmd_ctx, char *cmd, char retval = target->type->read_memory(target, address, 4, this_run_size / 4, buffer); if (retval != ERROR_OK) { - command_print(cmd_ctx, "Reading memory failed %d", retval); break; } - fileio_write(&fileio, this_run_size, buffer, &size_written); + retval = fileio_write(&fileio, this_run_size, buffer, &size_written); + if (retval != ERROR_OK) + { + break; + } size -= this_run_size; address += this_run_size; @@ -1989,7 +1992,10 @@ int handle_dump_image_command(struct command_context_s *cmd_ctx, char *cmd, char fileio_close(&fileio); duration_stop_measure(&duration, &duration_text); - command_print(cmd_ctx, "dumped %"PRIi64" byte in %s", fileio.size, duration_text); + if (retval==ERROR_OK) + { + command_print(cmd_ctx, "dumped %"PRIi64" byte in %s", fileio.size, duration_text); + } free(duration_text); return ERROR_OK; @@ -2045,6 +2051,7 @@ int handle_verify_image_command(struct command_context_s *cmd_ctx, char *cmd, ch } image_size = 0x0; + retval=ERROR_OK; for (i = 0; i < image.num_sections; i++) { buffer = malloc(image.sections[i].size); @@ -2055,11 +2062,8 @@ int handle_verify_image_command(struct command_context_s *cmd_ctx, char *cmd, ch } if ((retval = image_read_section(&image, i, 0x0, image.sections[i].size, buffer, &buf_cnt)) != ERROR_OK) { - ERROR("image_read_section failed with error code: %i", retval); - command_print(cmd_ctx, "image reading failed, verify aborted"); free(buffer); - image_close(&image); - return ERROR_OK; + break; } /* calculate checksum of image */ @@ -2069,10 +2073,8 @@ int handle_verify_image_command(struct command_context_s *cmd_ctx, char *cmd, ch if( retval != ERROR_OK ) { - command_print(cmd_ctx, "could not calculate checksum, verify aborted"); free(buffer); - image_close(&image); - return ERROR_OK; + break; } if( checksum != mem_checksum ) @@ -2104,8 +2106,8 @@ int handle_verify_image_command(struct command_context_s *cmd_ctx, char *cmd, ch command_print(cmd_ctx, "Verify operation failed address 0x%08x. Was 0x%02x instead of 0x%02x\n", t + image.sections[i].base_address, data[t], buffer[t]); free(data); free(buffer); - image_close(&image); - return ERROR_OK; + retval=ERROR_FAIL; + goto done; } } } @@ -2116,14 +2118,17 @@ int handle_verify_image_command(struct command_context_s *cmd_ctx, char *cmd, ch free(buffer); image_size += buf_cnt; } - +done: duration_stop_measure(&duration, &duration_text); - command_print(cmd_ctx, "verified %u bytes in %s", image_size, duration_text); + if (retval==ERROR_OK) + { + command_print(cmd_ctx, "verified %u bytes in %s", image_size, duration_text); + } free(duration_text); image_close(&image); - return ERROR_OK; + return retval; } int handle_bp_command(struct command_context_s *cmd_ctx, char *cmd, char **args, int argc)