Re: [PATCH 04/16] push remote_desc into struct remote_state

Pedro Alves
2013-06-24T17:24:25Z

This is OK.

A comment on something that caught my eye:

On 06/21/2013 06:24 PM, Tom Tromey wrote:
> @@ -10194,8 +10202,9 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
> FILE *file;
> gdb_byte *buffer;
> ULONGEST offset;
> + struct remote_state *rs = get_remote_state ();
>
> - if (!remote_desc)
> + if (!rs->remote_desc)
> error (_("command can only be used with remote target"));

This looks conceptually fishy.

A rs == NULL check would be less surprising. After all, if
we were multi-target already, and not using the remote target,
get_remote_state would most naturally return NULL.

But if not using the remote target, we probably shouldn't
be getting here anyway.

remote_file_get could nowadays be using the target_fileio_XXX methods
instead of remote_hostio_XXX, and therefore the command could be
generalized to work with all targets.

Something to keep in mind, and file under fix-it-later... Not sure
we have tests for these commands that try them when not connected to
a remote target.

--
Pedro Alves