Re: [PATCH 14/18] WTX-TCL support module

[Tom Tromey <tromey@redhat.com>]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> For VxWorks 5.x and 653, we need to use the TCL extension in order to
Joel> access some of the information we are looking for (list of VxWorks
Joel> tasks running on the target, for instance).  This is only because
Joel> the WTX protocol does not provide access to this info.

Joel> +/* Includes from the VxWorks install.  */
Joel> +#define HOST
Joel> +#include "tcl.h"
Joel> +#if WTX_PROT_VERSION != 4
Joel> +#include "wtxtcl.h"
Joel> +#endif

I guess this explains why the new code isn't part of
--enable-targets=all.  I am somewhat concerned that this will lead to
bit-rot.  I always build using this option in an attempt to avoid
breaking things; but this code will not be included in that.

I don't really see a way around it, though, unless you want to
virtualize all the wtx calls.

I guess I want us to be clear that build breakage for this is expected.

Joel> +static int
Joel> +wtxtcl_eval_verbose (char *str)
Joel> +{
Joel> +  char *output;
Joel> +  const int success = wtxtcl_eval (str, &output);
Joel> +
Joel> +  if (success)
Joel> +    printf_filtered ("%s\n", output);
Joel> +  else
Joel> +    printf_filtered (_("TCL error: %s\n"), output);
Joel> +
Joel> +  return success;
Joel> +}
Joel> +
Joel> +/* Implement the "tcl" command.  */
Joel> +
Joel> +static void
Joel> +tcl_command (char *args, int from_tty)
Joel> +{
Joel> +  if (args == NULL)
Joel> +    return;
Joel> +
Joel> +  wtxtcl_eval_verbose (args);

Why not call error instead of just printing a message when a Tcl command
fails?

Joel> +  tcl_cmd = xstrprintf ("taskInfoGet 0x%x", task_id);
Joel> +  success = wtxtcl_eval (tcl_cmd, &task_info);

Nothing ever frees tcl_cmd.

Joel> +  /* Skip the first 8 tokens and go directly to the 9th, which contains
Joel> +     the PD ID.  */
Joel> +  for (j = 0; j < 8; j++)
Joel> +    task_info = skip_space_delimited_token (task_info);

This is the wrong way to parse a Tcl list.  It may work ok for your
purposes, if you know that the result can never include anything "weird".
But it is easy and safer to just use Tcl_SplitList.

Joel> +  /* Parse the result.  */
Joel> +
Joel> +  current_thread = skip_whitespace (tcl_output);
Joel> +  while (current_thread && *current_thread != '\0')
Joel> +    {
Joel> +      struct wtxapi_thread_info *new_thread
Joel> +        = xmalloc (sizeof (struct wtxapi_thread_info));
Joel> +
Joel> +      /* Get the thread id.  */
Joel> +      new_thread->id = strtoul (current_thread, NULL, 0);
Joel> +      current_thread = skip_space_delimited_token (current_thread);
Joel> +
Joel> +      /* Get the thread name.  */
Joel> +      current_thread = skip_whitespace (current_thread);
Joel> +      if (*current_thread == '{')
Joel> +        {
Joel> +          /* The thread name delimited by curly braces.  Find the
Joel> +             closing curly brace.  */
Joel> +          char *start = current_thread + 1;  /* skip the '{'...  */
Joel> +          char *end = skip_until_character (current_thread, '}');
Joel> +          char tmp = *end;

Likewise.

Joel> +  add_com ("tcl", class_obscure, tcl_command,
Joel> +           _("Evaluate the arguments with the TCL interpreter"));

I am not super fond of a top-level command named "tcl".

Tom