[Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread gerrit
This is an automated email from Gerrit.

?yvind Harboe (oyvindhar...@gmail.com) just uploaded a new patch set to Gerrit, 
which you can find at http://openocd.zylin.com/130

-- gerrit

commit e0439139e9f1cc6d811c6f9bce49a2e36f01163e
Author: Øyvind Harboe oyvind.har...@zylin.com
Date:   Thu Oct 27 23:51:50 2011 +0200

bugfixes: numerous bugs in error propagation found by clang

Change-Id: I784068325b422d1918e28c08544bc5a1332d712f
Signed-off-by: Øyvind Harboe oyvind.har...@zylin.com

diff --git a/src/target/target.c b/src/target/target.c
index d4cb577..bd15620 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -1863,11 +1863,9 @@ int target_write_u8(struct target *target, uint32_t 
address, uint8_t value)
 
 COMMAND_HANDLER(handle_targets_command)
 {
-   struct target *target = all_targets;
-
if (CMD_ARGC == 1)
{
-   target = get_target(CMD_ARGV[0]);
+   struct target *target = get_target(CMD_ARGV[0]);
if (target == NULL) {
command_print(CMD_CTX,Target: %s is unknown, try one 
of:\n, CMD_ARGV[0]);
goto DumpTargets;
@@ -1882,9 +1880,9 @@ COMMAND_HANDLER(handle_targets_command)
CMD_CTX-current_target = target-target_number;
return ERROR_OK;
}
-DumpTargets:
+DumpTargets:;
 
-   target = all_targets;
+   struct target *target = all_targets;
command_print(CMD_CTX, TargetName Type   Endian 
TapNameState   );
command_print(CMD_CTX, --  -- -- -- 
-- );
while (target)
@@ -2190,6 +2188,8 @@ COMMAND_HANDLER(handle_reg_command)
}
}
 
+   assert(reg != NULL); /* give clang a hint that we *know* reg is != NULL 
here */
+
/* display a register */
if ((CMD_ARGC == 1) || ((CMD_ARGC == 2)  !((CMD_ARGV[1][0] = '0')  
(CMD_ARGV[1][0] = '9'
{
@@ -2210,6 +2210,8 @@ COMMAND_HANDLER(handle_reg_command)
if (CMD_ARGC == 2)
{
uint8_t *buf = malloc(DIV_ROUND_UP(reg-size, 8));
+   if (buf == NULL)
+   return ERROR_FAIL;
str_to_buf(CMD_ARGV[1], strlen(CMD_ARGV[1]), buf, reg-size, 0);
 
reg-type-set(reg, buf);
@@ -3414,9 +3416,9 @@ COMMAND_HANDLER(handle_profile_command)
/* hopefully it is safe to cache! We want to stop/restart as quickly as 
possible. */
struct reg *reg = register_get_by_name(target-reg_cache, pc, 1);
 
+   int retval = ERROR_OK;
for (;;)
{
-   int retval;
target_poll(target);
if (target-state == TARGET_HALTED)
{
@@ -3469,7 +3471,7 @@ COMMAND_HANDLER(handle_profile_command)
}
free(samples);
 
-   return ERROR_OK;
+   return retval;
 }
 
 static int new_int_array_element(Jim_Interp * interp, const char *varname, int 
idx, uint32_t val)
@@ -3634,7 +3636,7 @@ static int target_mem2array(Jim_Interp *interp, struct 
target *target, int argc,
Jim_SetResult(interp, Jim_NewEmptyStringObj(interp));
Jim_AppendStrings(interp, Jim_GetResult(interp), 
mem2array: cannot read memory, NULL);
e = JIM_ERR;
-   len = 0;
+   break;
} else {
v = 0; /* shut up gcc */
for (i = 0 ;i  count ;i++, n++) {
@@ -3659,7 +3661,7 @@ static int target_mem2array(Jim_Interp *interp, struct 
target *target, int argc,
 
Jim_SetResult(interp, Jim_NewEmptyStringObj(interp));
 
-   return JIM_OK;
+   return e;
 }
 
 static int get_int_array_element(Jim_Interp * interp, const char *varname, int 
idx, uint32_t *val)
@@ -3844,7 +3846,7 @@ static int target_array2mem(Jim_Interp *interp, struct 
target *target,
Jim_SetResult(interp, Jim_NewEmptyStringObj(interp));
Jim_AppendStrings(interp, Jim_GetResult(interp), 
array2mem: cannot read memory, NULL);
e = JIM_ERR;
-   len = 0;
+   break;
}
}
 
@@ -3852,7 +3854,7 @@ static int target_array2mem(Jim_Interp *interp, struct 
target *target,
 
Jim_SetResult(interp, Jim_NewEmptyStringObj(interp));
 
-   return JIM_OK;
+   return e;
 }
 
 /* FIX? should we propagate errors here rather than printing them
@@ -4164,6 +4166,8 @@ static int target_configure(Jim_GetOptInfo *goi, struct 
target *target)
free((void *)(target-variant));
}
e = Jim_GetOpt_String(goi, cp, NULL);
+   if (e != JIM_OK)
+   return e;
target-variant = strdup(cp);
 

Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread Peter Stuge
ger...@openocd.zylin.com wrote:
 -DumpTargets:
 +DumpTargets:;

Hm?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread Øyvind Harboe
2011/10/28 Peter Stuge pe...@stuge.se:
 ger...@openocd.zylin.com wrote:
 -DumpTargets:
 +DumpTargets:;

 Hm?

Syntactically a declaration can't follow a label, so add an empty statement.


-- 
Øyvind Harboe - Can Zylin Consulting help on your project?
US toll free 1-866-980-3434
http://www.zylin.com/
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread Peter Stuge
Øyvind Harboe wrote:
 2011/10/28 Peter Stuge pe...@stuge.se:
  ger...@openocd.zylin.com wrote:
  -DumpTargets:
  +DumpTargets:;
 
  Hm?
 
 Syntactically a declaration can't follow a label, so add an empty
 statement.

I don't like the change so much. It ends up declaring the exact same
variable twice, and other odd noise, to silence a warning. Is there
no better way?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread Øyvind Harboe
On Fri, Oct 28, 2011 at 12:23 AM, Peter Stuge pe...@stuge.se wrote:
 Øyvind Harboe wrote:
 2011/10/28 Peter Stuge pe...@stuge.se:
  ger...@openocd.zylin.com wrote:
  -DumpTargets:
  +DumpTargets:;
 
  Hm?

 Syntactically a declaration can't follow a label, so add an empty
 statement.

 I don't like the change so much. It ends up declaring the exact same
 variable twice, and other odd noise, to silence a warning. Is there
 no better way?

Reusing the 'target' variable name here is bad.

They are really two very distinct variable lifetimes.

The scope of the variables are now reduced.

The code is now more easily re-factored as multiple functions
because the same variable isn't being reused over and over for
difference purposes.


-- 
Øyvind Harboe - Can Zylin Consulting help on your project?
US toll free 1-866-980-3434
http://www.zylin.com/
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread Peter Stuge
Øyvind Harboe wrote:
  I don't like the change so much. It ends up declaring the exact same
  variable twice, and other odd noise, to silence a warning. Is there
  no better way?
 
 Reusing the 'target' variable name here is bad.
..
 The code is now more easily re-factored as multiple functions

Ok, can we please do that then, instead of working around static
analysis? Also, I'm not sure the command handler should really
return ERROR_OK if the target is unknown. (Not a new bug, but I
think it's a bug just the same.)


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

2011-10-27 Thread Øyvind Harboe
On Fri, Oct 28, 2011 at 12:59 AM, Peter Stuge pe...@stuge.se wrote:
 Øyvind Harboe wrote:
  I don't like the change so much. It ends up declaring the exact same
  variable twice, and other odd noise, to silence a warning. Is there
  no better way?

 Reusing the 'target' variable name here is bad.
 ..
 The code is now more easily re-factored as multiple functions

 Ok, can we please do that then, instead of working around static
 analysis?

In this case, I disagree that it is working around static analysis.

I'll see about having  a peek at the code again for cleaning it up.


-- 
Øyvind Harboe - Can Zylin Consulting help on your project?
US toll free 1-866-980-3434
http://www.zylin.com/
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development