Re: [RFA] remote debugging patches

[Michal Ludvig <mludvig@suse.cz>]

Daniel Jacobowitz wrote:
> But you had #define WAIT_FOREVER_FLAG 0.

During the weekend I was debugging problems in communication between 
gdb(i386) and gdbserver(x8664). Whenever I was stepping through 
gdbserver, the other side timeouted. I've found, that last argument to 
getpkt() is called 'forever', but in all calls was set to '0'. I didn't 
want these timeouts, so I changed all occurences of 0 to 
WAIT_FOREVER_FLAG, which could be set in compile-time. Most users and 
developpers (unless they will work on remote.c or alike) will leave this 
unchanged to 0, but sometimes it may be handy to set to 1 and recompile.

> - "no" and "yes" are useless values for a flag; they don't indicate any
> meaning.

Along with the name of the variable they have a meaning, I think. Of 
course, I can change them to 'wait'/'nowait' or 'forewer'/'timeout' or 
whatever else.

> - You made wait_forever_flag a variable that was never changed, and
> replaced a constant 0 with it... no point.

Who cares? In the debugger you'll see 'yes' or 'no' when you ask for the 
content of wait_forever_flag. Or do 'print /d wait_forever_flag' and you 
will se the decimal value.

> I think what Andrew had in mind was more like
> 
> enum {
>   do_not_wait_forever = 0,
>   wait_forever = 1
> };
> 
> and change calls to
>   getpkt (blah, do_not_wait_forever)

I don't understand the point of this :-( Then I'd have to change all 
occurences of do_not_wait_forever to wait_forever in the whole file to 
change the behaviour? Isn't it much easier to change just one line on 
top of the file from 'no' to 'yes' instead?
Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* +420 2 9654 5373   * http://www.suse.cz