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

[Doug Evans <dje@google.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?