RE: symbolic debug of loadable modules with kgdb light

[Caz Yokoyama <cazyokoyama at gmail dot com>]

Hello Joel,
I understand
1) You don't want to introduce dependency of the target into gdb
2) You want to check whether the user enters correct setting to
interrupt_sequence.
In case of debugging user space program, interrupt_sequence is always ^C.
However, when we debug kernel, interrupt_sequence is varied from kernel to
kernel. It means interrupt_sequence depends on each kernel. To check
correctness of interrupt_sequence, we have to know which kernel we are
debugging. It introduces dependency of kernel. As the result, it is
impossible to satisfy both 1) and 2). I give up. Thank you for reviewing my
patches.
-caz

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Monday, September 28, 2009 6:59 PM
To: caz yokoyama
Cc: Pedro Alves; gdb-patches@sourceware.org
Subject: Re: symbolic debug of loadable modules with kgdb light

> > I revised my patch. As I wrote in ChangeLog, I expanded
interrupt_sequence
> > to arbitrary characters include ^C and BREAK instead of 3 choices.

Actually, I'm sorry, but I don't agree with this change. I don't think
it's providing much in terms of user experience and your implementation
has many downsides: The command hander no longer verifies that the user
enters valid values (the user finds this out later when connecting to
the target or possibly only when requesting an interrupt; either way,
it's too late), you lose tab-completion, and your implementation of
send_interrupt_sequence becomes unecessarily complex and inefficient as
a result.

> -static int remote_break;
> +static int remote_break = 0;
> +
> +static void
> +set_remote_break (char *args, int from_tty, struct cmd_list_element *c)
> +{
> +  if (strcmp (interrupt_sequence, CTRL_C) == 0)
> +    {
> +      interrupt_sequence = xmalloc (sizeof(BREAK) + 1);
> +      strcpy (interrupt_sequence, BREAK);
> +      remote_break = 1;
> +    }
> +  else
> +    {
> +      interrupt_sequence = xmalloc (sizeof(CTRL_C) + 1);
> +      strcpy (interrupt_sequence, CTRL_C);
> +      remote_break = 0;
> +    }
> +}

What we are trying to achieve, here, is make things work for a user
that was used to using "set remotebreak" while yet telling him which
command should now be used instead. In other words, "set remotebreak on"
should act as if the user entered "set remote interrupt-sequence BREAK"
and "set remotebreak off" should act as if the user entered "set remote
interrupt-sequence control-c".

The code itself should *ignore* the boolean that set/show remotebreak
use internally, and use the "enum" that "set remote interrupt-sequence"
uses instead.

Calling deprecate_cmd on the cmd_list_element structs associcated to
the set/show remotebreak commands will make sure that the user is
informed which commands are replacing these commands, and allows us
to remove them eventually, once users have had a chance to transition
to the new ones.

-- 
Joel