[PING**4] [PATCH] Guile: temporary breakpoints

[Simon Marchi]

On 2021-06-08 5:00 p.m., Keith Seitz via Gdb-patches wrote:
> Hi,
> 
> I apologize that no one has yet reviewed your patch, which certainly seems
> like something we would want. Feature parity between Python and Guile can
> only be good for users.
> 
> I've nearly no experience with Guile, but looking this over, this patch looks
> pretty straightforward, but I do have some small suggestions/requests below.
> 
> Otherwise, I would like to recommend that a maintainer approve this.
> [I am not someone that can grant permission to push to the repo.]
> 
> Thanks again for the patch *and* your patience.
> 
> Keith

Thanks for looking at it Keith, it helps a lot.

>>>>>> +proc_with_prefix test_bkpt_temporary { } {
>>>>>> +    global srcfile testfile hex decimal
>>>>>> +
>>>>>> +    with_test_prefix test_bkpt_temporary {
>>>>>> +	# Start with a fresh gdb.
>>>>>> +	clean_restart ${testfile}
>>>>>> +
>>>>>> +	if ![gdb_guile_runto_main] then {
>>>>>> +	    fail "cannot run to main."
> 
> For consistency, please use "if {expr} {".
> 
> This result should probably be 'untested "cannot run to main"' instead
> of simply a fail. [Taken straight from gdb.base/template.exp.]

Perhaps it was a mistake to use untested in gdb.base/template.exp?

There are more instances of using "fail":


    $ ag -A 2 runto_main | grep untested | wc -l
    198
    $ ag -A 2 runto_main | grep fail | wc -l
    555

... and it makes more sense to me to use "fail".  If I introduce a bug
that prevents running to main to work, untested would be easier to miss
than fail.

Otherwise, I don't have the original context to quote but where it says:

  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
			bp_smob->bp->disposition == disp_del_at_next_stop);

put the || operator on the next line:

  return scm_from_bool (bp_smob->bp->disposition == disp_del
			|| bp_smob->bp->disposition == disp_del_at_next_stop);


Other than that, the patch LGTM after taking Keith's feedback in
consideration.

Simon