Code Review / openocd.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | review | tree
raw | patch | inline | side by side (from parent 1: bc1340c)
RTOS: Unify wipe-out of thread list 16/1916/8
authorChristian Eggers <ceggers@gmx.de>
Sat, 1 Feb 2014 09:17:17 +0000 (10:17 +0100)
committerSpencer Oliver <spen@spen-soft.co.uk>
Tue, 4 Mar 2014 20:13:58 +0000 (20:13 +0000)
Each RTOS implementation uses it's own (similar) code to free
the thread list. There are some additional issues:

<--->
if (pointer != NULL)
free(pointer);
<--->
This is not necessary, free(NULL) is perfectly ok.

<--->
free(rtos->thread_details);
rtos->thread_details = NULL;
rtos->thread_count = 0;
<--->
The 3rd line has been missing for all RTOS but ChibiOs. There are paths
in the code where rtos->thread_count is never set to NULL, which can
lead to null pointer dereference of rtos->thread_details.

Change-Id: I6f7045c3d4518b925cb80dd5c907a566536b34ad
Signed-off-by: Christian Eggers <ceggers@gmx.de>
---
Changelog:
v7:
- rtos_wipe_threadlist() --> rtos_free_threadlist()
- removed non related changes in gdb_server.c from this patch
v3:
- Removed world "topic" from first line of commit message
v2:
- typo: "whipe" --> "wipe"
Reviewed-on: http://openocd.zylin.com/1916
Tested-by: jenkins
Reviewed-by: Spencer Oliver <spen@spen-soft.co.uk>
src/rtos/ChibiOS.c patch | blob | history
src/rtos/FreeRTOS.c patch | blob | history
src/rtos/ThreadX.c patch | blob | history
src/rtos/eCos.c patch | blob | history
src/rtos/embKernel.c patch | blob | history
src/rtos/rtos.c patch | blob | history
src/rtos/rtos.h patch | blob | history

diff --git a/src/rtos/ChibiOS.c b/src/rtos/ChibiOS.c
index 2148e91d5277002910a326f9b3089c6d6d573e86..c54f43053f04fce5d4f2e083ce7d32243ed2a9f6 100644 (file)
--- a/src/rtos/ChibiOS.c
+++ b/src/rtos/ChibiOS.c
@@ -296,21 +296,8 @@ static int ChibiOS_update_threads(struct rtos *rtos)
        }
 
        /* wipe out previous thread details if any */
        }
 
        /* wipe out previous thread details if any */
-       int j;
-       if (rtos->thread_details) {
-               for (j = 0; j < rtos->thread_count; j++) {
-                       struct thread_detail *current_thread = &rtos->thread_details[j];
-                       if (current_thread->display_str != NULL)
-                               free(current_thread->display_str);
-                       if (current_thread->thread_name_str != NULL)
-                               free(current_thread->thread_name_str);
-                       if (current_thread->extra_info_str != NULL)
-                               free(current_thread->extra_info_str);
-               }
-               free(rtos->thread_details);
-               rtos->thread_details = NULL;
-               rtos->thread_count = 0;
-       }
+       rtos_free_threadlist(rtos);
+
        /* ChibiOS does not save the current thread count. We have to first
         * parse the double linked thread list to check for errors and the number of
         * threads. */
        /* ChibiOS does not save the current thread count. We have to first
         * parse the double linked thread list to check for errors and the number of
         * threads. */
diff --git a/src/rtos/FreeRTOS.c b/src/rtos/FreeRTOS.c
index 321b1e12c5e62ec8a0e0cf633250e33c3b0a3677..598e2d6627caebaf77ea5c6915abc3c7e354cf17 100644 (file)
--- a/src/rtos/FreeRTOS.c
+++ b/src/rtos/FreeRTOS.c
@@ -173,25 +173,7 @@ static int FreeRTOS_update_threads(struct rtos *rtos)
        }
 
        /* wipe out previous thread details if any */
        }
 
        /* wipe out previous thread details if any */
-       if (rtos->thread_details != NULL) {
-               int j;
-               for (j = 0; j < rtos->thread_count; j++) {
-                       if (rtos->thread_details[j].display_str != NULL) {
-                               free(rtos->thread_details[j].display_str);
-                               rtos->thread_details[j].display_str = NULL;
-                       }
-                       if (rtos->thread_details[j].thread_name_str != NULL) {
-                               free(rtos->thread_details[j].thread_name_str);
-                               rtos->thread_details[j].thread_name_str = NULL;
-                       }
-                       if (rtos->thread_details[j].extra_info_str != NULL) {
-                               free(rtos->thread_details[j].extra_info_str);
-                               rtos->thread_details[j].extra_info_str = NULL;
-                       }
-               }
-               free(rtos->thread_details);
-               rtos->thread_details = NULL;
-       }
+       rtos_free_threadlist(rtos);
 
        /* read the current thread */
        retval = target_read_buffer(rtos->target,
 
        /* read the current thread */
        retval = target_read_buffer(rtos->target,
diff --git a/src/rtos/ThreadX.c b/src/rtos/ThreadX.c
index 19dc46360220a113cf96ee8bc6cc270ef4774a54..add34b4f4db8b2c2da8cb3dd0032e8d74317422c 100644 (file)
--- a/src/rtos/ThreadX.c
+++ b/src/rtos/ThreadX.c
@@ -155,25 +155,7 @@ static int ThreadX_update_threads(struct rtos *rtos)
        }
 
        /* wipe out previous thread details if any */
        }
 
        /* wipe out previous thread details if any */
-       if (rtos->thread_details != NULL) {
-               int j;
-               for (j = 0; j < rtos->thread_count; j++) {
-                       if (rtos->thread_details[j].display_str != NULL) {
-                               free(rtos->thread_details[j].display_str);
-                               rtos->thread_details[j].display_str = NULL;
-                       }
-                       if (rtos->thread_details[j].thread_name_str != NULL) {
-                               free(rtos->thread_details[j].thread_name_str);
-                               rtos->thread_details[j].thread_name_str = NULL;
-                       }
-                       if (rtos->thread_details[j].extra_info_str != NULL) {
-                               free(rtos->thread_details[j].extra_info_str);
-                               rtos->thread_details[j].extra_info_str = NULL;
-                       }
-               }
-               free(rtos->thread_details);
-               rtos->thread_details = NULL;
-       }
+       rtos_free_threadlist(rtos);
 
        /* read the current thread id */
        retval = target_read_buffer(rtos->target,
 
        /* read the current thread id */
        retval = target_read_buffer(rtos->target,
diff --git a/src/rtos/eCos.c b/src/rtos/eCos.c
index 9ab88dec01e5da06b633040a9540808e5a16c73a..7310d6d64039be212284fe18f50a77488ac1a3b3 100644 (file)
--- a/src/rtos/eCos.c
+++ b/src/rtos/eCos.c
@@ -125,25 +125,7 @@ static int eCos_update_threads(struct rtos *rtos)
        }
 
        /* wipe out previous thread details if any */
        }
 
        /* wipe out previous thread details if any */
-       if (rtos->thread_details != NULL) {
-               int j;
-               for (j = 0; j < rtos->thread_count; j++) {
-                       if (rtos->thread_details[j].display_str != NULL) {
-                               free(rtos->thread_details[j].display_str);
-                               rtos->thread_details[j].display_str = NULL;
-                       }
-                       if (rtos->thread_details[j].thread_name_str != NULL) {
-                               free(rtos->thread_details[j].thread_name_str);
-                               rtos->thread_details[j].thread_name_str = NULL;
-                       }
-                       if (rtos->thread_details[j].extra_info_str != NULL) {
-                               free(rtos->thread_details[j].extra_info_str);
-                               rtos->thread_details[j].extra_info_str = NULL;
-                       }
-               }
-               free(rtos->thread_details);
-               rtos->thread_details = NULL;
-       }
+       rtos_free_threadlist(rtos);
 
        /* determine the number of current threads */
        uint32_t thread_list_head = rtos->symbols[eCos_VAL_thread_list].address;
 
        /* determine the number of current threads */
        uint32_t thread_list_head = rtos->symbols[eCos_VAL_thread_list].address;
diff --git a/src/rtos/embKernel.c b/src/rtos/embKernel.c
index f605deb3c5bd77386be1f39e1d48abc97bd66017..76c0bd23b2857722cfd7e1464ba8acfe5d2be16b 100644 (file)
--- a/src/rtos/embKernel.c
+++ b/src/rtos/embKernel.c
@@ -206,25 +206,7 @@ static int embKernel_update_threads(struct rtos *rtos)
        }
 
        /* wipe out previous thread details if any */
        }
 
        /* wipe out previous thread details if any */
-       if (rtos->thread_details != NULL) {
-               int j;
-               for (j = 0; j < rtos->thread_count; j++) {
-                       if (rtos->thread_details[j].display_str != NULL) {
-                               free(rtos->thread_details[j].display_str);
-                               rtos->thread_details[j].display_str = NULL;
-                       }
-                       if (rtos->thread_details[j].thread_name_str != NULL) {
-                               free(rtos->thread_details[j].thread_name_str);
-                               rtos->thread_details[j].thread_name_str = NULL;
-                       }
-                       if (rtos->thread_details[j].extra_info_str != NULL) {
-                               free(rtos->thread_details[j].extra_info_str);
-                               rtos->thread_details[j].extra_info_str = NULL;
-                       }
-               }
-               free(rtos->thread_details);
-               rtos->thread_details = NULL;
-       }
+       rtos_free_threadlist(rtos);
 
        param = (const struct embKernel_params *) rtos->rtos_specific_params;
 
 
        param = (const struct embKernel_params *) rtos->rtos_specific_params;
 
diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index cdd37608ed446156ca409fd61405b8427b170ab3..0082ced1ff0bf8c12401ede09f796af4af5ce8d0 100644 (file)
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -513,3 +513,20 @@ int rtos_update_threads(struct target *target)
                target->rtos->type->update_threads(target->rtos);
        return ERROR_OK;
 }
                target->rtos->type->update_threads(target->rtos);
        return ERROR_OK;
 }
+
+void rtos_free_threadlist(struct rtos *rtos)
+{
+       if (rtos->thread_details) {
+               int j;
+
+               for (j = 0; j < rtos->thread_count; j++) {
+                       struct thread_detail *current_thread = &rtos->thread_details[j];
+                       free(current_thread->display_str);
+                       free(current_thread->thread_name_str);
+                       free(current_thread->extra_info_str);
+               }
+               free(rtos->thread_details);
+               rtos->thread_details = NULL;
+               rtos->thread_count = 0;
+       }
+}
diff --git a/src/rtos/rtos.h b/src/rtos/rtos.h
index 12a96d2ab0e1b439b4975d822569eeec7503ea44..f8aa33f10b23152656c0dc955138e6db8ab8e769 100644 (file)
--- a/src/rtos/rtos.h
+++ b/src/rtos/rtos.h
@@ -98,6 +98,7 @@ int rtos_try_next(struct target *target);
 int gdb_thread_packet(struct connection *connection, char *packet, int packet_size);
 int rtos_get_gdb_reg_list(struct connection *connection);
 int rtos_update_threads(struct target *target);
 int gdb_thread_packet(struct connection *connection, char *packet, int packet_size);
 int rtos_get_gdb_reg_list(struct connection *connection);
 int rtos_update_threads(struct target *target);
+void rtos_free_threadlist(struct rtos *rtos);
 int rtos_smp_init(struct target *target);
 /*  function for handling symbol access */
 int rtos_qsymbol(struct connection *connection, char *packet, int packet_size);
 int rtos_smp_init(struct target *target);
 /*  function for handling symbol access */
 int rtos_qsymbol(struct connection *connection, char *packet, int packet_size);
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)