On Thu, Jun 17, 2010 at 7:24 AM, Øyvind Harboe <[email protected]> wrote:
> On Thu, Jun 17, 2010 at 2:05 AM, Andreas Fritiofson
> <[email protected]> wrote:
>> On Thu, Jun 17, 2010 at 12:30 AM, Øyvind Harboe <[email protected]>
>> wrote:
>>> On Wed, Jun 16, 2010 at 11:40 PM, Andreas Fritiofson
>>> <[email protected]> wrote:
>>>>
>>>> The stack trace provides no valuable information to the user for
>>>> interactive commands.
>>>
>>> What about nested proc's?
>>>
>>
>> You mean when calling user defined tcl procedures calling other tcl
>> procedures that fails?
>>
>> My guess is the *user* doesn't particularly care about the chain of
>> events leading up to the fault. It's probably due to either misuse of
>> the first procedure, in which case the user is fully aware of what the
>> called procedure was, or a bug in one of the following. If it's a bug
>> it calls for debugging, a job for a *developer* (might of course be
>> the same person with another hat). The developer could flip the debug
>> level switch and see the stack trace as previously.
>>
>> Then on the other hand I don't get a stack trace when a shell script
>> in multiple nesting levels fails, and I'm not very bothered about
>> that.
>>
>> I think the current error messages do more harm than good in the
>> majority of cases. If there's a better solution I'm open to
>> suggestions.
>
> I agree that the current error messages are ugly, but I don't
> agree that we should not print file and line # of e.g. syntax
> error.
The problem is that the file and line# printed is the same for all
syntax errors and failed builtins and has no relevance to the actual
error. A useful trace is only produced when a nested tcl procedure
fails on a lower level. I haven't actually noticed that feature before
but now when I've played around with it a bit I agree that we want to
keep it.
> However, we may be able to address this in a more fine
> grained fashion than simply turning the stack traces on
> and off.
>
> Perhaps you could post some examples and show what
> parts of the stack trace that is useful and what's just (debug)
> noise?
For example, misspelling a subcommand:
> flash prob 0
flash prob 0: command requires more arguments <-- 1)
in procedure 'flash' called at file "command.c", line 654 <-- 2)
called at file "command.c", line 365 <-- 3)
1) This is nonsense, but not really related to the stack trace.
2) I didn't call the flash command from command.c, i called it from an
interactive telnet session. Nothing flash related can be found at line
654.
3) Same as 2, but worse. What was called at line 365??
In short, there's no useful part of this stack trace.
A more interesting example, a nested command that fails:
$ openocd -f bitsbytes.tcl -c "normalize_bitfield 1 2 q"
...
Runtime error, file "bitsbytes.tcl", line 31: <-- 4)
Syntax error in expression
in procedure 'normalize_bitfield' called at file "command.c", line 654 <-- 5)
in procedure 'extract_bitfield' called at file "bitsbytes.tcl", line 50 <-- 6)
in procedure 'create_mask' called at file "bitsbytes.tcl", line 39
This is where I start to see the point of the stack trace. However,
it's not correct.
4) there's no normalize_bitfield at line 31 as it's suggested.
5) same problem as 2, I called it from the command line.
6) extract_bitfield is really called at line 50, but NOT in procedure
create_mask. Line 50 is in normalize_bitfield.
It struck me that the stack trace would make much more sense if it was
written in the reverse order! Let's see:
Runtime error, file "bitsbytes.tcl", line 31:
Syntax error in expression
in procedure 'create_mask' called at file "bitsbytes.tcl", line 39
in procedure 'extract_bitfield' called at file "bitsbytes.tcl", line 50
in procedure 'normalize_bitfield' called at file "command.c", line 654
Apart from the command.c reference this looks quite alright!
I attach two patches. One to remove the command.c references and one
to rearrange the stack trace output. Any takers?
/Andreas
From 9ebd0e62a8112ec227d11f0633fc7a897217b8b8 Mon Sep 17 00:00:00 2001
From: Andreas Fritiofson <[email protected]>
Date: Fri, 18 Jun 2010 00:48:47 +0200
Subject: [PATCH 1/2] don't add confusing source info to Jim
When an interactive command fails, the Jim stack trace prints references to
the line in "command.c" where the interpreter was invoked. Since that
location has no relation to the actual command that failed, the information
serves only to add confusion.
By not adding the useless source info to Jim the noise can be reduced,
while still printing a useful trace for nested commands.
Signed-off-by: Andreas Fritiofson <[email protected]>
---
src/helper/command.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/helper/command.c b/src/helper/command.c
index be262f2..ea768b2 100644
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -362,7 +362,7 @@ static int register_command_handler(struct command_context *cmd_ctx,
if (NULL == override_name)
return JIM_ERR;
- retval = Jim_Eval_Named(interp, override_name, __THIS__FILE__ , __LINE__);
+ retval = Jim_Eval_Named(interp, override_name, 0, 0);
free((void *)override_name);
return retval;
@@ -651,7 +651,7 @@ int command_run_line(struct command_context *context, char *line)
retcode = Jim_SetAssocData(interp, "retval", NULL, &retval);
if (retcode == JIM_OK)
{
- retcode = Jim_Eval_Named(interp, line, __THIS__FILE__, __LINE__);
+ retcode = Jim_Eval_Named(interp, line, 0, 0);
Jim_DeleteAssocData(interp, "retval");
}
--
1.6.3.3
From 17ae0641d674232902759ba6eba4679c7c3e5215 Mon Sep 17 00:00:00 2001
From: Andreas Fritiofson <[email protected]>
Date: Fri, 18 Jun 2010 01:33:22 +0200
Subject: [PATCH 2/2] reverse order of Jim stack trace output
The stack traces makes much more sense this way.
Signed-off-by: Andreas Fritiofson <[email protected]>
---
src/helper/jim.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/helper/jim.c b/src/helper/jim.c
index 8538954..3964421 100644
--- a/src/helper/jim.c
+++ b/src/helper/jim.c
@@ -12314,7 +12314,7 @@ void Jim_PrintErrorMessage(Jim_Interp *interp)
Jim_fprintf(interp,interp->cookie_stderr, "%s" JIM_NL,
Jim_GetString(interp->result, NULL));
Jim_ListLength(interp, interp->stackTrace, &len);
- for (i = len-3; i >= 0; i-= 3) {
+ for (i = 0; i < len; i += 3) {
Jim_Obj *objPtr=NULL;
const char *proc, *file, *line;
--
1.6.3.3
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development
