This is an automated email from Gerrit.

Tomas Vanek ([email protected]) just uploaded a new patch set to Gerrit, which you 
can find at http://openocd.zylin.com/4871

-- gerrit

commit 578d1957ed27377e99f6f26c8ae9d87aa5be713b
Author: Tomas Vanek <[email protected]>
Date:   Thu Jan 24 14:33:16 2019 +0100

    target, breakpoints: improve error handling
    
    handle_bp_command_set() showed the error mesage
    "Failure setting breakpoint, the same address(IVA) is already used"
    on any error returned from (xxx_)breakpoint_add().
    Paradoxically breakpoint_add() returned ERROR_OK if it detected
    duplicated bp addres.
    context_breakpoint_add() and hybrid_breakpoint_add() returned -1
    instead of OpenOCD compatible error if they detected duplicity.
    
    Introduce ERROR_TARGET_DUPLICATE_BREAKPOINT
    Unify error handling to LOG_ERROR() any error in (xxx_)breakpoint_add()
    Remove misleading error messages from handle_bp_command_set()
    handle_bp_command_set() returns error if the target does not implement
    add_context_breakpoint or add_hybrid_breakpoint.
    
    Change-Id: If17dfad1756d82a77028ebdc4b305f9c8e1365ba
    Signed-off-by: Tomas Vanek <[email protected]>

diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c
index 58bcc86..94e8a82 100644
--- a/src/target/breakpoints.c
+++ b/src/target/breakpoints.c
@@ -60,9 +60,9 @@ int breakpoint_add_internal(struct target *target,
                         * breakpoint" ... check all the parameters before
                         * succeeding.
                         */
-                       LOG_DEBUG("Duplicate Breakpoint address: " 
TARGET_ADDR_FMT " (BP %" PRIu32 ")",
+                       LOG_ERROR("Duplicate Breakpoint address: " 
TARGET_ADDR_FMT " (BP %" PRIu32 ")",
                                address, breakpoint->unique_id);
-                       return ERROR_OK;
+                       return ERROR_TARGET_DUPLICATE_BREAKPOINT;
                }
                breakpoint_p = &breakpoint->next;
                breakpoint = breakpoint->next;
@@ -124,9 +124,9 @@ int context_breakpoint_add_internal(struct target *target,
                         * breakpoint" ... check all the parameters before
                         * succeeding.
                         */
-                       LOG_DEBUG("Duplicate Breakpoint asid: 0x%08" PRIx32 " 
(BP %" PRIu32 ")",
+                       LOG_ERROR("Duplicate Breakpoint asid: 0x%08" PRIx32 " 
(BP %" PRIu32 ")",
                                asid, breakpoint->unique_id);
-                       return -1;
+                       return ERROR_TARGET_DUPLICATE_BREAKPOINT;
                }
                breakpoint_p = &breakpoint->next;
                breakpoint = breakpoint->next;
@@ -176,13 +176,13 @@ int hybrid_breakpoint_add_internal(struct target *target,
                         * breakpoint" ... check all the parameters before
                         * succeeding.
                         */
-                       LOG_DEBUG("Duplicate Hybrid Breakpoint asid: 0x%08" 
PRIx32 " (BP %" PRIu32 ")",
+                       LOG_ERROR("Duplicate Hybrid Breakpoint asid: 0x%08" 
PRIx32 " (BP %" PRIu32 ")",
                                asid, breakpoint->unique_id);
-                       return -1;
+                       return ERROR_TARGET_DUPLICATE_BREAKPOINT;
                } else if ((breakpoint->address == address) && 
(breakpoint->asid == 0)) {
-                       LOG_DEBUG("Duplicate Breakpoint IVA: " TARGET_ADDR_FMT 
" (BP %" PRIu32 ")",
+                       LOG_ERROR("Duplicate Breakpoint IVA: " TARGET_ADDR_FMT 
" (BP %" PRIu32 ")",
                                address, breakpoint->unique_id);
-                       return -1;
+                       return ERROR_TARGET_DUPLICATE_BREAKPOINT;
 
                }
                breakpoint_p = &breakpoint->next;
diff --git a/src/target/target.c b/src/target/target.c
index b4bf5d3..c28909b 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -3719,38 +3719,31 @@ static int handle_bp_command_set(struct command_context 
*cmd_ctx,
 
        if (asid == 0) {
                retval = breakpoint_add(target, addr, length, hw);
+               /* error is always logged in breakpoint_add(), do not print it 
again */
                if (ERROR_OK == retval)
                        command_print(cmd_ctx, "breakpoint set at " 
TARGET_ADDR_FMT "", addr);
-               else {
-                       LOG_ERROR("Failure setting breakpoint, the same 
address(IVA) is already used");
-                       return retval;
-               }
+
        } else if (addr == 0) {
                if (target->type->add_context_breakpoint == NULL) {
-                       LOG_WARNING("Context breakpoint not available");
-                       return ERROR_OK;
+                       LOG_ERROR("Context breakpoint not available");
+                       return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
                }
                retval = context_breakpoint_add(target, asid, length, hw);
+               /* error is always logged in context_breakpoint_add(), do not 
print it again */
                if (ERROR_OK == retval)
                        command_print(cmd_ctx, "Context breakpoint set at 
0x%8.8" PRIx32 "", asid);
-               else {
-                       LOG_ERROR("Failure setting breakpoint, the same 
address(CONTEXTID) is already used");
-                       return retval;
-               }
+
        } else {
                if (target->type->add_hybrid_breakpoint == NULL) {
-                       LOG_WARNING("Hybrid breakpoint not available");
-                       return ERROR_OK;
+                       LOG_ERROR("Hybrid breakpoint not available");
+                       return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
                }
                retval = hybrid_breakpoint_add(target, addr, asid, length, hw);
+               /* error is always logged in hybrid_breakpoint_add(), do not 
print it again */
                if (ERROR_OK == retval)
                        command_print(cmd_ctx, "Hybrid breakpoint set at 
0x%8.8" PRIx32 "", asid);
-               else {
-                       LOG_ERROR("Failure setting breakpoint, the same address 
is already used");
-                       return retval;
-               }
        }
-       return ERROR_OK;
+       return retval;
 }
 
 COMMAND_HANDLER(handle_bp_command)
diff --git a/src/target/target.h b/src/target/target.h
index fb9d714..983f450 100644
--- a/src/target/target.h
+++ b/src/target/target.h
@@ -725,6 +725,7 @@ void target_handle_event(struct target *t, enum 
target_event e);
 #define ERROR_TARGET_TRANSLATION_FAULT (-309)
 #define ERROR_TARGET_NOT_RUNNING (-310)
 #define ERROR_TARGET_NOT_EXAMINED (-311)
+#define ERROR_TARGET_DUPLICATE_BREAKPOINT (-312)
 
 extern bool get_target_reset_nag(void);
 

-- 


_______________________________________________
OpenOCD-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openocd-devel

Reply via email to