Re: [Openocd-development] [PATCH 1/3] command: the Jim interpreter can now be provided rather than created
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
#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
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
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
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
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
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
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
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