Re: [patch] gdbserver: Add support for Z0/Z1 packets

[Aleksandar Ristovski <aristovski@qnx.com>]

Pedro Alves wrote:
> On Wednesday 24 June 2009 19:50:44, Aleksandar Ristovski wrote:
>> Pedro Alves wrote:
>>> On Tuesday 23 June 2009 16:17:58, Aleksandar Ristovski wrote:
>>>> Pedro Alves wrote:
>>>>> On Monday 22 June 2009 20:38:50, Aleksandar Ristovski wrote:
>>>>>
>>>>>>> Z0 and Z1 breakpoints also take a 'len' argument, just
>>>>>>> like Z2-Z4.  You should also pass those down.
>>>>>>>
>>>>>>> But, Let's take a step back --- why not just rename the
>>>>>>> insert_watchpoint|remove_watchpoint functions to insert_point,remove_point,
>>>>>>> and relax the type checks in server.c:
>>>>>> But either way is fine with me - just let me know.
>>>>> I'd prefer the approach I suggested, and worry about splitting
>>>>> the breakpoints from watchpoints API if/when we actually need it.
>>>>>
>>>> Ok, then that version is committed.
>>> Well, we had never seen "that" version
>> Ok, to rectify this I am attaching two versions: one if I 
>> revert the changes I committed and the other is diff to what 
>> is in now.
>>
>>  > ... and you bypassed the "rename" suggestion...
>>
>> I did not do any renaming - I think it is not terribly 
>> confusing since both in target.h comment and server.c 'Z' 
>> case it is made very clear that it handles both breakpoints 
>> and watchpoints (i.e. I don't find it any clearer if it was 
>> called "insert_point"... it would still require reading the 
>> comment in target.h)
> 
> 
> Urgh, I was just about to press the send button when I saw
> this message of yours.  This version corrects a few troubles
> with the previous commit (see ChangeLog) (one of them I still
> see in your new patch) and I've tested it on x86_64-linux.
> Aleksandar, please, please, do run the testsuite (and state
> that you have) when posting patches.  E.g., we could have caught
> the vKill issue that Pierre fixed when we made only linux
> report multi-process (the testsuite runs in "target remote"
> mode, hence with multi-process off most of the way).
> 


Your patch: Wasn't it more desireable to distinguish between 
(1) -unsupported and
(-1)-error?

i.e. in "default", shouldn't it be "res=-1" so that it makes 
it clear that gdb passed invalid argument?


Testsuite: I do run it but I admit sometimes not due to 
issues my system is giving me lately; I have to run tests 
"manually" i.e. one by one (I'll have to take some time to 
figure out why is the test run, after some time, just 
starting to ERROR, unable to properly spawn new gdb instance).



-- 
Aleksandar Ristovski
QNX Software Systems