Re: MI/CLI breakpoint handling code duplication

[Vladimir Prus <ghost@cs.msu.su>]

Nick Roberts wrote:

> [2nd attempt]
> 
> (Fri, 14 Dec 2007 10:26:49 +0400)
> 
>> > * breakpoint.c (break_command_really): New, copied
>> > from break_command_1. New parameters COND_STRING, THREAD
>> > PARSE_CONDITITION_AND_THREAD and PENDING_BREAK_SUPPORT.
>> > The previous FLAG parameter split into TEMPFLAG and
>> > HARDWAREFLAG.
>> > When PARSE_CONDITION_AND_THREAD is not set, duplicate
>> > the passed condition string.
>> > (struct captured_breakpoint_args): Remove
>> > (do_captured_breakpoint): Remove.
>> > (break_command_1): Relay to break_command_really.
>> > (gdb_breakpoint): Relay to break_command_really.
> 
>> Ping? This patch is pure refactoring which is basis for
>> the 'pending breakpoint in MI' patch. Given that the patch
>> is not supposed to change any behaviour and causes no regression,
>> I'd hope it should be easy to review :-)
> 
> Previously:
> 
>   (gdb)
>   -break-insert sqrt
>   &"Function \"sqrt\" not defined.\n"
>   ^error,msg="Function \"sqrt\" not defined."
>   (gdb)
> 
> 
> After this change:
> 
>   (gdb)
>   -break-insert sqrt
>   &"Function \"sqrt\" not defined.\n"
>   ^error,msg="unknown error"
>   (gdb)
> 
> 
> So this is *not* pure refactoring and happens because gdb_breakpoint used
> catch_exceptions_with_msg previously while it now uses catch_exception
> through break_command_really.  This means now that in mi_cmd_break_insert
> 
> 
>       rc = gdb_breakpoint (address, condition,
> 0 /*hardwareflag */ , temp_p,
> thread, ignore_count,
> pending,
> &mi_error_message);
> 
> mi_error_message points to NULL.
> 
> 
> Furthermore with a sequence like:
> 
> (gdb)
> -break-insert main
> ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080485bc",func="main",file="/home/nickrob/myprog.c",fullname="/home/nickrob/myprog.c",line="79",times="0"}
> (gdb)
> -break-insert sqrt
> &"Function \"sqrt\" not defined.\n"
> ^error,msg="unknown error"
> (gdb)
> -exec-run
> ^running
> (gdb)
> *stopped,bkpt={number="-1",type="longjmp
> resume",disp="keep",enabled="n",addr="0x00000000",at="",times="0"},bkpt={number="-2",type="longjmp
> resume",disp="keep",enabled="n",addr="0x00000000",at="",times="0"},bkpt={number="-4",type="longjmp
> resume",disp="keep",enabled="n",addr="0x00000000",at="",times="0"},bkpt={number="-5",type="longjmp
> resume",disp="keep",enabled="n",addr="0x00000000",at="",times="0"},reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x080485bc",func="main",args=[{name="argc",value="-1074490904"},{name="argv",value="0xbff49214"}],file="/home/nickrob/myprog.c",fullname="/home/nickrob/myprog.c",line="79"}
> (gdb)
> 
> 
> GDB prints extra output because previous content has not been flushed from
> the
> stream.  I think this is because gdb_breakpoint tries to return int
> (really enum REASON, -2 or -1) through break_command_really while it is
> type enum gdb_rc (0, 1 or 2).
> 
> So that when gdb_breakpoint fails (and returns RETURN_ERROR = -1)
> mi_cmd_break_insert thinks it has suceeded and returns MI_CMD_DONE
> (GDB_RC_FAIL = 0):
> 
>   if (rc == GDB_RC_FAIL)
>     return MI_CMD_ERROR;
>   else
>     return MI_CMD_DONE;
> 
> I still don't understand the difference between breakpoint handling in MI
> and CLI but it must surely have been done differently for a reason.

I don't understand the last sentence, but I'll look into fixing the issues
you've reported.

- Volodya