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

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> @@ -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"));

Pedro> This looks conceptually fishy.

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

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

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

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

Thanks Pedro.

You saw the future quite accurately here.  I did see crashes in this
code after the multi-target conversion, precisely because I changed
get_remote_state to return NULL when not connected; and this path is
exercised by the test suite.

So, a later patch will introduce the == NULL check as well.

In this series, though, get_remote_state cannot return NULL.  This may
seem odd, but it is consistent with the state of the code before the
series -- and I wanted to try to make this series obviously not
behavior-changing.

Tom