This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Unset attach_flag when running a new process


On Thu, Aug 13, 2015 at 12:30 PM, Pedro Alves <palves@redhat.com> wrote:
> On 08/02/2015 09:20 PM, Patrick Palka wrote:
>> On Thu, Jul 30, 2015 at 12:24 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
>>> On 30/07/15 16:28, Patrick Palka wrote:
>>>>
>>>> +  /* Unset attach_flag, it may be set if the previous process associated
>>>> with
>>>> +     the inferior was attached to.  */
>>>> +  current_inferior ()->attach_flag = 0;
>>>
>>>
>>> It is better to do such reset in target.c:target_pre_inferior, which
>>> is created for such purpose, as far as I know.
>>
>> That makes sense, although I am a bit hesitant about such a change,
>> since target_pre_inferior has a lot of callers.
>
> You mean, the whole lot of 3 callers.  :-)
>
> infcmd.c:542:  target_pre_inferior (from_tty);
> infcmd.c:2603:  target_pre_inferior (from_tty);
> target.c:2191:  target_pre_inferior (from_tty);

The thing is that one of the callers, target_preopen, itself has lots
of callers all over the place.

>
>> Is it OK, for
>> instance, to assume that current_inferior points to the right inferior
>> when target_pre_inferior gets called?  Currently current_inferior is
>> not called directly or indirectly from target_pre_inferior.
>
> Should be.  It's called from the run command, and from the
> attach command.  Those assume that the current inferior is
> the inferior that we're about to run/attach.

Yeah, those are obviously fine.

>
> The call in target_preopen is more dubious.  That should probably
> be iterating over all inferiors and calling target_pre_inferior
> on each.  For the case where you have multiple inferiors, and then
> connect to a remote target with "target remote".

I see.  I will move the call to target_pre_inferior then.

>
>
>
>> +set test "kill process"
>> +send_gdb "kill\n"
>> +gdb_expect {
>> +    -re ".y or n. $" {
>> +     send_gdb "y\n"
>> +     exp_continue
>> +    }
>> +    -re "$gdb_prompt $" {
>> +     pass $test
>> +    }
>> +    default {
>> +     fail $test
>> +    }
>> +}
>
> Avoid raw gdb_expect.  You can write:
>
>         gdb_test "kill" \
>             "" \
>             "kill process" \
>             "Kill the program being debugged.*y or n. $" \
>             "y"

Cool, didn't know about that.

>
>> +
>> +set test "restart process"
>> +gdb_test "start" "Starting program.*Temporary breakpoint .*" $test
>> +
>> +set test "attempt kill via quit"
>> +send_gdb "quit\n"
>> +set ok 0
>> +# The quit prompt should warn about killing the process, not about detaching the
>> +# process, since this process was not attached to.
>> +gdb_expect {
>
> Please use gdb_test_multiple.  That catches internal errors and such.

Hmm, I wonder why I didn't use gdb_test_multiple in the first place.
Probably for no good reason.  I'll make this change then.

>
>> +    -re "will be killed.*.y or n. $" {
>> +     set ok 1
>> +     send_gdb "n\n"
>> +     exp_continue
>> +    }
>> +    -re "will be detached.*.y or n. $" {
>> +     send_gdb "n\n"
>> +     exp_continue
>> +    }
>> +    -re "$gdb_prompt $" {
>> +     if $ok {
>> +         pass $test
>> +     } else {
>> +         fail $test
>> +     }
>
> gdb_assert $ok $test

Done.

>
>
>> +    }
>> +    default {
>> +     fail $test
>> +    }
>> +}
>> +
>> +remote_exec host "kill $testpid"
>>
>
> Thanks,
> Pedro Alves
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]