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

[Pedro Alves <palves at redhat dot com>]

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