Re: [PATCH] Do not pass NULL for the string in catch_errors

[Luis Machado <lgustavo@codesourcery.com>]

On 10/22/2015 11:43 AM, Pedro Alves wrote:
> On 10/22/2015 01:36 PM, Luis Machado wrote:
>> On 10/22/2015 09:50 AM, Pedro Alves wrote:
>>> On 10/22/2015 12:23 PM, Luis Machado wrote:
>
>> That would be fine by me. I was just experimenting with
>> TRY/CATCH/END_CATCH after my unsuccessful replacement of catch_errors
>> with catch_exceptions. See below.
>>>>
>
>>>> With catch_exceptions, instead of catching the error and letting the
>>>> inferior continue, it will just cause the inferior to terminate.
>>>
>>> I don't understand.  Why do you say this will happen?
>>>
>>
>> I replaced catch_errors with catch_exceptions in record-full.c. I saw a
>> bunch of failures in gdb.reverse/sigall-reverse.exp, starting at this point:
>>
>> Breakpoint 142, handle_TERM (sig=15) at
>> ../../../gdb-head-ro/gdb/testsuite/gdb.reverse/sigall-reverse.c:378^M
>> 378     }^M
>> (gdb) PASS: gdb.reverse/sigall-reverse.exp: send signal TERM
>> continue^M
>> Continuing.^M
>> The next instruction is syscall exit_group.  It will make the program
>> exit.  Do you want to stop the program?([y] or n) yes^M
>> Process record: inferior program stopped.^M
>> ^M
>> [process 21188] #1 stopped.^M
>>
>> The above is a normal run. If i replace catch_errors with
>> catch_exceptions, instead of stopping the inferior, it will terminate.
>> Maybe there is a bug somewhere, or something is being mishandled.
>
> It just sounds to me that you didn't take into account
> that the return values of catch_errors and catch_exceptions
> differ.
>
> while one does:
>
>    if (exception.reason < 0)
>      {
> ...
>        return exception.reason;
>      }
>
> the other does:
>
>    if (exception.reason != 0)
>      return 0;
>
> This matters because the result is returned by
> record_full_message_wrapper_safe, and checked here:
>
> 		      if (!record_full_message_wrapper_safe (regcache,
> 							     GDB_SIGNAL_0))
>    			{
>                             status->kind = TARGET_WAITKIND_STOPPED;
>                             status->value.sig = GDB_SIGNAL_0;
>                             break;
>    			}
>

Indeed this is the case. I think i'll keep catch_errors and only fix the 
NULL parameter then. Having to adjust return values from unrelated 
functions sounds error-prone and maybe not worth it if we're moving away 
from these types of constructs in the future.