- Fixed various error handling when looking for memory leaks
authoroharboe <oharboe@b42882b7-edfa-0310-969c-e2dbd0fdcd60>
Mon, 10 Mar 2008 12:11:07 +0000 (12:11 +0000)
committeroharboe <oharboe@b42882b7-edfa-0310-969c-e2dbd0fdcd60>
Mon, 10 Mar 2008 12:11:07 +0000 (12:11 +0000)
- 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

src/server/gdb_server.c
src/target/target.c

index f01d638fddd2cac05e5c5b9818e66c2e463b1ded..ce45f2980abf0aac7b699b5129ac490b453d091f 100644 (file)
@@ -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)
index dc69e994438f4e821bf85bbf80fb66d70c2ec34a..51aef65e94949b927700a1051abb2961ea6d1263 100644 (file)
@@ -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, "<target> <run time ms>");
        register_command(cmd_ctx, NULL, "working_area", handle_working_area_command, COMMAND_ANY, "working_area <target#> <address> <size> <'backup'|'nobackup'> [virtual address]");
        register_command(cmd_ctx, NULL, "virt2phys", handle_virt2phys_command, COMMAND_ANY, "virt2phys <virtual address>");
 
@@ -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: <type> <endianess> <reset_mode>");
-               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)

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)