Re: [PATCH v2] gdb: tests: set remotetimeout to gdb_load_timeout for remote targets

Jie Zhang
2011-06-29T17:29:39Z

On 06/29/2011 12:25 PM, Joel Brobecker wrote:
>> Rather than relying on the default remotetimeout value (which might be
>> too small for some slower devices), use the existing gdb_load_timeout
>> config option to set it.
>
> No one seems to be comfortable looking at this, so I took a look.
> I think the idea makes sense.
>
Thank you! I can only answer the following question and will leave
others to Mike.

>> + if [is_remote target] {
>> + set oldremotetimeout [get_remotetimeout]
>> + set_remotetimeout $loadtimeout
>> + }
>
> Is there a specific reason you already know about that made you
> change the remotetimeout only when the target is remote? If there is,
> I think it'd be nice to add a comment to that effect. If it's just
> because we don't expect the timeout to have any effect in the non-
> remote case, then I guess we can leave it like that. But I'm not
> to certain about that. I would consider the idea of making things
> simpler by just setting the timeout everytime, since this part of
> the code will always result in a "load" command being used.
>
I think it makes no sense to change remotetimeout for non-remote case so
I added that conditional. I would like to leave it like that since I
think the code looks clearer in this way. If we take it away, the code
is simpler, but someone looking at the code might wonder why set
remotetimeout for non-remote target.

Regards,
Jie