Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Zach Welch
Only one minor suggested improvement, below.  Otherwise, these both look
okay to me.

On Tue, 2009-12-01 at 08:48 +0100, Øyvind Harboe wrote:
 In embedded hosts, the Jim interpreter can come from the
 existing context rather than be created by OpenOCD.
 
 Signed-off-by: Øyvind Harboe oyvind.har...@zylin.com
 ---
  src/helper/command.c |   18 +++---
  src/helper/command.h |6 --
  src/openocd.c|6 +++---
  3 files changed, 18 insertions(+), 12 deletions(-)
 
 diff --git a/src/helper/command.c b/src/helper/command.c
 index dcad6a1..d657668 100644
 --- a/src/helper/command.c
 +++ b/src/helper/command.c
 @@ -1272,7 +1272,7 @@ static const struct command_registration 
 command_builtin_handlers[] = {
   COMMAND_REGISTRATION_DONE
  };
  
 -struct command_context* command_init(const char *startup_tcl)
 +struct command_context* command_init(const char *startup_tcl, Jim_Interp 
 *interp)
  {
   struct command_context* context = malloc(sizeof(struct 
 command_context));
   const char *HostOs;
 @@ -1284,14 +1284,18 @@ struct command_context* command_init(const char 
 *startup_tcl)
   context-output_handler_priv = NULL;
  
  #if !BUILD_ECOSBOARD

It is now safe to kill this #if logic too.  It's like a bonus prize. ;)

 - Jim_InitEmbedded();
 - /* Create an interpreter */
 - context-interp = Jim_CreateInterp();
 - /* Add all the Jim core commands */
 - Jim_RegisterCoreCommands(context-interp);
 + /* Create a jim interpreter if we were not handed one */
 + if (interp == NULL)
 + {
 + Jim_InitEmbedded();
 + /* Create an interpreter */
 + interp = Jim_CreateInterp();
 + /* Add all the Jim core commands */
 + Jim_RegisterCoreCommands(interp);
 + }
  #endif
 + context-interp = interp;
  
 - Jim_Interp *interp = context-interp;
  #if defined(_MSC_VER)
   /* WinXX - is generic, the forward
* looking problem is this:
 diff --git a/src/helper/command.h b/src/helper/command.h
 index 611db87..8d68c18 100644
 --- a/src/helper/command.h
 +++ b/src/helper/command.h
 @@ -323,9 +323,11 @@ void command_set_output_handler(struct command_context* 
 context,
  int command_context_mode(struct command_context *context, enum command_mode 
 mode);
  
  /**
 - * Creates a new command context using the startup TCL provided.
 + * Creates a new command context using the startup TCL provided and
 + * the existing Jim interpreter, if any. If interp == NULL, then command_init
 + * creates a command interpreter.
   */
 -struct command_context* command_init(const char *startup_tcl);
 +struct command_context* command_init(const char *startup_tcl, Jim_Interp 
 *interp);
  /**
   * Creates a copy of an existing command context.  This does not create
   * a deep copy of the command list, so modifications in one context will
 diff --git a/src/openocd.c b/src/openocd.c
 index 22d4582..44e0292 100644
 --- a/src/openocd.c
 +++ b/src/openocd.c
 @@ -188,14 +188,14 @@ static const struct command_registration 
 openocd_command_handlers[] = {
  struct command_context *global_cmd_ctx;
  
  /* NB! this fn can be invoked outside this file for non PC hosted builds */
 -struct command_context *setup_command_handler(void)
 +struct command_context *setup_command_handler(Jim_Interp *interp)
  {
   log_init();
   LOG_DEBUG(log_init: complete);
  
   struct command_context *cmd_ctx;
  
 - global_cmd_ctx = cmd_ctx = command_init(openocd_startup_tcl);
 + global_cmd_ctx = cmd_ctx = command_init(openocd_startup_tcl, interp);
  
   register_commands(cmd_ctx, NULL, openocd_command_handlers);
   /* register subsystem commands */
 @@ -242,7 +242,7 @@ int openocd_main(int argc, char *argv[])
   /* initialize commandline interface */
   struct command_context *cmd_ctx;
  
 - cmd_ctx = setup_command_handler();
 + cmd_ctx = setup_command_handler(NULL);
  
  #if BUILD_IOUTIL
   if (ioutil_init(cmd_ctx) != ERROR_OK)


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


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Øyvind Harboe
  #if !BUILD_ECOSBOARD

 It is now safe to kill this #if logic too.  It's like a bonus prize. ;)

Almost, but not quite. I tried before I remembered that
Jim is embedded into the athttpd server... You know the
slightly messy and awkward jump tables that Jim uses...

I'll push the two patches soonish.

-- 
Øyvind Harboe
US toll free 1-866-980-3434 / International +47 51 63 25 00
http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Zach Welch
On Tue, 2009-12-01 at 09:17 +0100, Øyvind Harboe wrote:
   #if !BUILD_ECOSBOARD
 
  It is now safe to kill this #if logic too.  It's like a bonus prize. ;)
 
 Almost, but not quite. I tried before I remembered that
 Jim is embedded into the athttpd server... You know the
 slightly messy and awkward jump tables that Jim uses...

That's not true.  It _can_ be removed.  If interp is NULL, then we dang
well had better create it right then and there, or the code will crash.

The patch to ecosboard.c always gives a non-NULL value, and we always
pass in NULL from openocd.c.  The #if is entirely redundant when you
take a moment to look at the big picture.

--Z

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


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Øyvind Harboe
 The patch to ecosboard.c always gives a non-NULL value, and we always
 pass in NULL from openocd.c.  The #if is entirely redundant when you
 take a moment to look at the big picture.

Except it doesn't build, it's that messy bit with difference build modes for
Jim.

I'm kinda hopeful that the #if can be removed eventually if Jim improves
to have a less obtuse way to handle embedded vs. using in include files...

-- 
Øyvind Harboe
US toll free 1-866-980-3434 / International +47 51 63 25 00
http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Zach Welch
On Tue, 2009-12-01 at 10:16 +0100, Øyvind Harboe wrote:
  The patch to ecosboard.c always gives a non-NULL value, and we always
  pass in NULL from openocd.c.  The #if is entirely redundant when you
  take a moment to look at the big picture.
 
 Except it doesn't build, it's that messy bit with difference build modes for
 Jim.
 
 I'm kinda hopeful that the #if can be removed eventually if Jim improves
 to have a less obtuse way to handle embedded vs. using in include files...

I have been meaning to produce a series that seriously cleans up the #if
madness that we have going on.  This nonsense drives me insane.  I have
a solution for this particular problem, so maybe it's time to start it.

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


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Øyvind Harboe
On Tue, Dec 1, 2009 at 10:19 AM, Zach Welch z...@superlucidity.net wrote:
 On Tue, 2009-12-01 at 10:16 +0100, Øyvind Harboe wrote:
  The patch to ecosboard.c always gives a non-NULL value, and we always
  pass in NULL from openocd.c.  The #if is entirely redundant when you
  take a moment to look at the big picture.

 Except it doesn't build, it's that messy bit with difference build modes for
 Jim.

 I'm kinda hopeful that the #if can be removed eventually if Jim improves
 to have a less obtuse way to handle embedded vs. using in include files...

 I have been meaning to produce a series that seriously cleans up the #if
 madness that we have going on.  This nonsense drives me insane.  I have
 a solution for this particular problem, so maybe it's time to start it.

W.r.t. Jim Tcl there be dragons. Btw, Jim Tcl is is trying to recruit
a new maintainer...

Are you sure you it's a good idea to start this now?

To truly tackle this you would have to handle different cases: jim tcl
in shared library, jim tcl statically linked, jim tcl created by someone
else than OpenOCD...

-- 
Øyvind Harboe
US toll free 1-866-980-3434 / International +47 51 63 25 00
http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-12-01 Thread Zach Welch
On Tue, 2009-12-01 at 10:24 +0100, Øyvind Harboe wrote:
 On Tue, Dec 1, 2009 at 10:19 AM, Zach Welch z...@superlucidity.net wrote:
  On Tue, 2009-12-01 at 10:16 +0100, Øyvind Harboe wrote:
   The patch to ecosboard.c always gives a non-NULL value, and we always
   pass in NULL from openocd.c.  The #if is entirely redundant when you
   take a moment to look at the big picture.
 
  Except it doesn't build, it's that messy bit with difference build modes 
  for
  Jim.
 
  I'm kinda hopeful that the #if can be removed eventually if Jim improves
  to have a less obtuse way to handle embedded vs. using in include files...
 
  I have been meaning to produce a series that seriously cleans up the #if
  madness that we have going on.  This nonsense drives me insane.  I have
  a solution for this particular problem, so maybe it's time to start it.
 
 W.r.t. Jim Tcl there be dragons. Btw, Jim Tcl is is trying to recruit
 a new maintainer...

If I took it over, I would fork and start on a new major revision.
The current version appears to be unmaintainable... which, incidentally,
might be why it needs a new maintainer. ;)

 Are you sure you it's a good idea to start this now?

Yes, but you are verily confused about my direction.

 To truly tackle this you would have to handle different cases: jim tcl
 in shared library, jim tcl statically linked, jim tcl created by someone
 else than OpenOCD...

For OpenOCD, it is not nearly as hard as you make it seem:

1) Rename ecosboard.c as openocd_ecos.c
2) Add openocd_common.c, openocd_linux.c, openocd_win32.c, etc.
3) Declare stuff in openocd.h and define them in all files.  Some files
will declare only stubs... others will

This would allow us to get rid of most Win32, eCos, and other
platform-related #if's throughout the tree, perhaps all of them.

However, the original patch from $SUBJECT could still be improved:
that #if block can be moved to openocd.c, which has the effect today as
the plan above would accomplish.  Thus, we can add 'assert(interp)' at
the top of the command_init routine, as both callers pass it in.

--Z

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


[Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-11-30 Thread Øyvind Harboe
In embedded hosts, the Jim interpreter can come from the
existing context rather than be created by OpenOCD.

Signed-off-by: Øyvind Harboe oyvind.har...@zylin.com
---
 src/helper/command.c |   18 +++---
 src/helper/command.h |6 --
 src/openocd.c|6 +++---
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/helper/command.c b/src/helper/command.c
index dcad6a1..d657668 100644
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -1272,7 +1272,7 @@ static const struct command_registration 
command_builtin_handlers[] = {
COMMAND_REGISTRATION_DONE
 };
 
-struct command_context* command_init(const char *startup_tcl)
+struct command_context* command_init(const char *startup_tcl, Jim_Interp 
*interp)
 {
struct command_context* context = malloc(sizeof(struct 
command_context));
const char *HostOs;
@@ -1284,14 +1284,18 @@ struct command_context* command_init(const char 
*startup_tcl)
context-output_handler_priv = NULL;
 
 #if !BUILD_ECOSBOARD
-   Jim_InitEmbedded();
-   /* Create an interpreter */
-   context-interp = Jim_CreateInterp();
-   /* Add all the Jim core commands */
-   Jim_RegisterCoreCommands(context-interp);
+   /* Create a jim interpreter if we were not handed one */
+   if (interp == NULL)
+   {
+   Jim_InitEmbedded();
+   /* Create an interpreter */
+   interp = Jim_CreateInterp();
+   /* Add all the Jim core commands */
+   Jim_RegisterCoreCommands(interp);
+   }
 #endif
+   context-interp = interp;
 
-   Jim_Interp *interp = context-interp;
 #if defined(_MSC_VER)
/* WinXX - is generic, the forward
 * looking problem is this:
diff --git a/src/helper/command.h b/src/helper/command.h
index 611db87..8d68c18 100644
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -323,9 +323,11 @@ void command_set_output_handler(struct command_context* 
context,
 int command_context_mode(struct command_context *context, enum command_mode 
mode);
 
 /**
- * Creates a new command context using the startup TCL provided.
+ * Creates a new command context using the startup TCL provided and
+ * the existing Jim interpreter, if any. If interp == NULL, then command_init
+ * creates a command interpreter.
  */
-struct command_context* command_init(const char *startup_tcl);
+struct command_context* command_init(const char *startup_tcl, Jim_Interp 
*interp);
 /**
  * Creates a copy of an existing command context.  This does not create
  * a deep copy of the command list, so modifications in one context will
diff --git a/src/openocd.c b/src/openocd.c
index 22d4582..44e0292 100644
--- a/src/openocd.c
+++ b/src/openocd.c
@@ -188,14 +188,14 @@ static const struct command_registration 
openocd_command_handlers[] = {
 struct command_context *global_cmd_ctx;
 
 /* NB! this fn can be invoked outside this file for non PC hosted builds */
-struct command_context *setup_command_handler(void)
+struct command_context *setup_command_handler(Jim_Interp *interp)
 {
log_init();
LOG_DEBUG(log_init: complete);
 
struct command_context *cmd_ctx;
 
-   global_cmd_ctx = cmd_ctx = command_init(openocd_startup_tcl);
+   global_cmd_ctx = cmd_ctx = command_init(openocd_startup_tcl, interp);
 
register_commands(cmd_ctx, NULL, openocd_command_handlers);
/* register subsystem commands */
@@ -242,7 +242,7 @@ int openocd_main(int argc, char *argv[])
/* initialize commandline interface */
struct command_context *cmd_ctx;
 
-   cmd_ctx = setup_command_handler();
+   cmd_ctx = setup_command_handler(NULL);
 
 #if BUILD_IOUTIL
if (ioutil_init(cmd_ctx) != ERROR_OK)
-- 
1.6.3.3

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


Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created

2009-11-30 Thread Øyvind Harboe
There is no third patch for this series, I just left out an unsuitable
patch and I guess I should have used different args to git
format patch to make it a series of two patches.

-- 
Øyvind Harboe
US toll free 1-866-980-3434 / International +47 51 63 25 00
http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development