watchpoint_add() cleanup

Fail watchpoint_add() if it's the same address but the
parameters are different ... don't just assume having
the same address means the same watchpoint!  (Note that
overlapping watchpoints aren't detected...)

Handle unrecognized return codes more sanely; don't exit()!
And describe command params right.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c
index 88986518543ca569bb2c7addfcfb32a2bfe71e16..dd672b9fef22d6aebd340a05054aa5cc600728e1 100644 (file)
--- a/src/target/breakpoints.c
+++ b/src/target/breakpoints.c
@@ -178,54 +178,61 @@ breakpoint_t* breakpoint_find(target_t *target, uint32_t address)
        return NULL;
 }
 
-int watchpoint_add(target_t *target, uint32_t address, uint32_t length, enum watchpoint_rw rw, uint32_t value, uint32_t mask)
+int watchpoint_add(target_t *target, uint32_t address, uint32_t length,
+               enum watchpoint_rw rw, uint32_t value, uint32_t mask)
 {
        watchpoint_t *watchpoint = target->watchpoints;
        watchpoint_t **watchpoint_p = &target->watchpoints;
        int retval;
+       char *reason;
 
        while (watchpoint)
        {
-               if (watchpoint->address == address)
+               if (watchpoint->address == address) {
+                       if (watchpoint->length != length
+                                       || watchpoint->value != value
+                                       || watchpoint->mask != mask
+                                       || watchpoint->rw != rw) {
+                               LOG_ERROR("address 0x%8.8" PRIx32
+                                               "already has watchpoint %d",
+                                               address, watchpoint->unique_id);
+                               return ERROR_FAIL;
+                       }
+
+                       /* ignore duplicate watchpoint */
                        return ERROR_OK;
+               }
                watchpoint_p = &watchpoint->next;
                watchpoint = watchpoint->next;
        }
 
-       (*watchpoint_p) = malloc(sizeof(watchpoint_t));
+       (*watchpoint_p) = calloc(1, sizeof(watchpoint_t));
        (*watchpoint_p)->address = address;
        (*watchpoint_p)->length = length;
        (*watchpoint_p)->value = value;
        (*watchpoint_p)->mask = mask;
        (*watchpoint_p)->rw = rw;
-       (*watchpoint_p)->set = 0;
-       (*watchpoint_p)->next = NULL;
        (*watchpoint_p)->unique_id = bpwp_unique_id++;
 
-       if ((retval = target_add_watchpoint(target, *watchpoint_p)) != ERROR_OK)
-       {
-               switch (retval)
-               {
-                       case ERROR_TARGET_RESOURCE_NOT_AVAILABLE:
-                               LOG_INFO("can't add %s watchpoint, resource not available (WPID: %d)",
-                                        watchpoint_rw_strings[(*watchpoint_p)->rw],
-                                        (*watchpoint_p)->unique_id );
-                               free (*watchpoint_p);
-                               *watchpoint_p = NULL;
-                               return retval;
-                               break;
-                       case ERROR_TARGET_NOT_HALTED:
-                               LOG_INFO("can't add watchpoint while target is running (WPID: %d)",
-                                                (*watchpoint_p)->unique_id );
-                               free (*watchpoint_p);
-                               *watchpoint_p = NULL;
-                               return retval;
-                               break;
-                       default:
-                               LOG_ERROR("unknown error");
-                               exit(-1);
-                               break;
-               }
+       retval = target_add_watchpoint(target, *watchpoint_p);
+       switch (retval) {
+       case ERROR_OK:
+               break;
+       case ERROR_TARGET_RESOURCE_NOT_AVAILABLE:
+               reason = "resource not available";
+               goto bye;
+       case ERROR_TARGET_NOT_HALTED:
+               reason = "target running";
+               goto bye;
+       default:
+               reason = "unrecognized error";
+bye:
+               LOG_ERROR("can't add %s watchpoint at 0x%8.8" PRIx32 ", %s",
+                        watchpoint_rw_strings[(*watchpoint_p)->rw],
+                        address, reason);
+               free (*watchpoint_p);
+               *watchpoint_p = NULL;
+               return retval;
        }
 
        LOG_DEBUG("added %s watchpoint at 0x%8.8" PRIx32 " of length 0x%8.8x (WPID: %d)",

diff --git a/src/target/breakpoints.h b/src/target/breakpoints.h
index b75aae62446505ecc0cacf198acaffdb63597382..91459669e26652c09b0958dd5aaefb81c03b680a 100644 (file)
--- a/src/target/breakpoints.h
+++ b/src/target/breakpoints.h
@@ -49,7 +49,7 @@ typedef struct breakpoint_s
 typedef struct watchpoint_s
 {
        uint32_t address;
-       int length;
+       uint32_t length;
        uint32_t mask;
        uint32_t value;
        enum watchpoint_rw rw;

diff --git a/src/target/cortex_m3.c b/src/target/cortex_m3.c
index eb37add3d2e39bb13b14cb84bf2a1dc6254e6934..51481a98873d0bb3eff42aa88ce586a2d759d6e2 100644 (file)
--- a/src/target/cortex_m3.c
+++ b/src/target/cortex_m3.c
@@ -1199,7 +1199,7 @@ cortex_m3_add_watchpoint(struct target_s *target, watchpoint_t *watchpoint)
        unsigned mask;
 
        for (mask = 0; mask < 16; mask++) {
-               if ((1 << mask) == watchpoint->length)
+               if ((1u << mask) == watchpoint->length)
                        break;
        }
        if (mask == 16) {

diff --git a/src/target/target.c b/src/target/target.c
index 2a66fcf107a8c3b6e831bcbaf2302614833b1a67..55fc2c8237ead31961d1a83aa5583bbcb2e5aeb9 100644 (file)
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -2988,7 +2988,8 @@ static int handle_wp_command(struct command_context_s *cmd_ctx, char *cmd, char
                break;
 
        default:
-               command_print(cmd_ctx, "usage: wp <address> <length> [r/w/a] [value] [mask]");
+               command_print(cmd_ctx, "usage: wp [address length "
+                               "[(r|w|a) [value [mask]]]]");
                return ERROR_COMMAND_SYNTAX_ERROR;
        }