This is an automated email from Gerrit.

Paul Fertser (fercer...@gmail.com) just uploaded a new patch set to Gerrit, 
which you can find at http://openocd.zylin.com/2541

-- gerrit

commit 277783b52a0574084ef53ef2cc2e8c5870ab76ba
Author: Paul Fertser <fercer...@gmail.com>
Date:   Wed Feb 11 10:51:17 2015 +0300

    target: fix timer callbacks processing
    
    Warning, behaviour change: before this patch if a timer callback
    returned an error, the other handlers in the list were not called.
    
    This patch fixes two different issues with the way timer callbacks are
    called:
    
    1. The function is not designed to be reentrant but a nested call is
    possible via: target_handle timer event -> poll -> target events
    before/after reexaminantion -> script_command_run ->
    target_call_timer_callbacks_now . This patch makes function a no-op
    when called recursively;
    
    2. The current code can deal with the case when calling a handler
    leads to its removal but not when it leads to removal of the next
    callback in the list. This patch defers actual removal to consolidate
    it with the calling loop.
    
    These bugs were exposed by Valgrind.
    
    Change-Id: Ia628a744634f5d2911eb329747e826cb9772e789
    Signed-off-by: Paul Fertser <fercer...@gmail.com>

diff --git a/src/target/target.c b/src/target/target.c
index af5c5b9..318bc50 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -1330,6 +1330,7 @@ int target_register_timer_callback(int (*callback)(void 
*priv), int time_ms, int
        (*callbacks_p)->callback = callback;
        (*callbacks_p)->periodic = periodic;
        (*callbacks_p)->time_ms = time_ms;
+       (*callbacks_p)->removed = false;
 
        gettimeofday(&now, NULL);
        (*callbacks_p)->when.tv_usec = now.tv_usec + (time_ms % 1000) * 1000;
@@ -1371,24 +1372,18 @@ int target_unregister_event_callback(int 
(*callback)(struct target *target,
 
 int target_unregister_timer_callback(int (*callback)(void *priv), void *priv)
 {
-       struct target_timer_callback **p = &target_timer_callbacks;
-       struct target_timer_callback *c = target_timer_callbacks;
-
        if (callback == NULL)
                return ERROR_COMMAND_SYNTAX_ERROR;
 
-       while (c) {
-               struct target_timer_callback *next = c->next;
+       for (struct target_timer_callback *c = target_timer_callbacks;
+            c; c = c->next) {
                if ((c->callback == callback) && (c->priv == priv)) {
-                       *p = next;
-                       free(c);
+                       c->removed = true;
                        return ERROR_OK;
-               } else
-                       p = &(c->next);
-               c = next;
+               }
        }
 
-       return ERROR_OK;
+       return ERROR_FAIL;
 }
 
 int target_call_event_callbacks(struct target *target, enum target_event event)
@@ -1442,31 +1437,41 @@ static int target_call_timer_callback(struct 
target_timer_callback *cb,
 
 static int target_call_timer_callbacks_check_time(int checktime)
 {
+       static bool callback_processing;
+
+       /* Do not allow nesting */
+       if (callback_processing)
+               return ERROR_OK;
+
+       callback_processing = true;
+
        keep_alive();
 
        struct timeval now;
        gettimeofday(&now, NULL);
 
-       struct target_timer_callback *callback = target_timer_callbacks;
-       while (callback) {
-               /* cleaning up may unregister and free this callback */
-               struct target_timer_callback *next_callback = callback->next;
+       struct target_timer_callback **callback = &target_timer_callbacks;
+       while (*callback) {
+               if ((*callback)->removed) {
+                       struct target_timer_callback *p = *callback;
+                       *callback = (*callback)->next;
+                       free(p);
+                       continue;
+               }
 
-               bool call_it = callback->callback &&
-                       ((!checktime && callback->periodic) ||
-                         now.tv_sec > callback->when.tv_sec ||
-                        (now.tv_sec == callback->when.tv_sec &&
-                         now.tv_usec >= callback->when.tv_usec));
+               bool call_it = (*callback)->callback &&
+                       ((!checktime && (*callback)->periodic) ||
+                        now.tv_sec > (*callback)->when.tv_sec ||
+                        (now.tv_sec == (*callback)->when.tv_sec &&
+                         now.tv_usec >= (*callback)->when.tv_usec));
 
-               if (call_it) {
-                       int retval = target_call_timer_callback(callback, &now);
-                       if (retval != ERROR_OK)
-                               return retval;
-               }
+               if (call_it)
+                       target_call_timer_callback(*callback, &now);
 
-               callback = next_callback;
+               callback = &(*callback)->next;
        }
 
+       callback_processing = false;
        return ERROR_OK;
 }
 
diff --git a/src/target/target.h b/src/target/target.h
index 0552b8f..fcb6963 100644
--- a/src/target/target.h
+++ b/src/target/target.h
@@ -286,6 +286,7 @@ struct target_timer_callback {
        int (*callback)(void *priv);
        int time_ms;
        int periodic;
+       bool removed;
        struct timeval when;
        void *priv;
        struct target_timer_callback *next;

-- 

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
OpenOCD-devel mailing list
OpenOCD-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openocd-devel

Reply via email to