Re: [PATCH 1/2] Warn when accessing binaries over RSP

[Doug Evans <dje at google dot com>]

Gary Benson writes:
 > Andrew Burgess wrote:
 > > * Gary Benson <gbenson@redhat.com> [2015-08-05 16:28:15 +0100]:
 > > >
 > > > diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
 > > > index 1781d80..b511777 100644
 > > > --- a/gdb/gdb_bfd.c
 > > > +++ b/gdb/gdb_bfd.c
 > > > @@ -219,13 +219,38 @@ gdb_bfd_iovec_fileio_open (struct bfd *abfd,  
void *inferior)
 > > >    const char *filename = bfd_get_filename (abfd);
 > > >    int fd, target_errno;
 > > >    int *stream;
 > > > +  struct target_ops *ops = find_target_at (process_stratum);
 > > >
 > > >    gdb_assert (is_target_filename (filename));
 > > > +  filename += strlen (TARGET_SYSROOT_PREFIX);
 > > > +
 > > > +  /* GDB provides no indicator of progress during file operations,  
and
 > > > +     can appear to have locked up during slow remote transfers, so  
we
 > > > +     inform the user what is happening and suggest a way out.  It's
 > > > +     unpleasant that we need to detect remote targets this way  
(rather
 > > > +     than putting the warnings in remote_hostio_open), but it's not
 > > > +     possible for remote_hostio_open to differentiate between
 > > > +     accessing inferior binaries (which can be bypassed) and  
accessing
 > > > +     things like /proc/ (which is unavoidable).  */
 > > > +  if (strcmp (ops->to_shortname, "remote") == 0
 > > > +      || strcmp (ops->to_shortname, "extended-remote") == 0)
 > > > +    {
 > > > +      static int warning_issued = 0;
 > > > +
 > > > +      printf_unfiltered (_("Reading %s from remote target\n"),
 > > > +			 filename);
 > > > +
 > > > +      if (!warning_issued)
 > > > +	{
 > > > +	  warning (_("File transfers from remote targets can be slow.\n"
 > > > +		     "Use \"set sysroot\" to access files locally"
 > > > +		     " instead."));
 > > > +	  warning_issued = 1;
 > > > +	}
 > > > +    }
 > >
 > > Altering the behaviour based on to_shortname feels like breaking
 > > this nice target OO model we have.
 >
 > Yeah... :-/
 >
 > > Could the warning not be moved down into target_fileio_open instead?
 >
 > Not so much target_fileio_open as remote_hostio_open; only remote
 > targets need the warning.  And originally I thought no, the warning
 > couldn't go there, because target_fileio_open/remote_hostio_open is
 > used for various internal things such as /proc/ file reads on Linux
 > that the user shouldn't see.
 >
 > ...however...
 >
 > remote_hostio_open *can* differentiate between reading inferior
 > binaries and reading internal stuff because the internal stuff is
 > accessed with the INF argument NULL and binaries are accessed with
 > a non-NULL INF.
 >
 > So I can do that, if it doesn't seem too hacky.
 >
 > > Or if that's really not an appropriate place should we add a new
 > > target method?
 >
 > I considered that but couldn't think of a good name :-)
 > target_fileio_warn_if_slow ??
 > I can do that too.

FAOD, target_fileio_open_warn_if_slow?