This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 04/16] push remote_desc into struct remote_state
- From: Tom Tromey <tromey at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 27 Jun 2013 14:21:36 -0600
- Subject: Re: [PATCH 04/16] push remote_desc into struct remote_state
- References: <1371835506-15691-1-git-send-email-tromey at redhat dot com> <1371835506-15691-5-git-send-email-tromey at redhat dot com> <51C880C5 dot 6050307 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