Re: [PATCH 2/7] Introduce target_fileio_set_fs

Gary Benson
2015-04-17T13:36:36Z

Doug Evans wrote:
> On Thu, Apr 16, 2015 at 5:19 AM, Gary Benson <gbenson@redhat.com> wrote:
> > This commit introduces a new target method target_fileio_set_fs
> > and adds calls to it in various places. This allows support to
> > be added for targets where processes do not share a common
> > filesystem.
> >
[snip]
> >
> > diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> > index 9d75b66..44b2970 100644
> > --- a/gdb/linux-tdep.c
> > +++ b/gdb/linux-tdep.c
> > @@ -715,6 +715,9 @@ linux_info_proc
> > if (args && args[0])
> > error (_("Too many parameters: %s"), args);
> >
> > + /* Access /proc/ from our filesystem. */
> > + target_fileio_set_fs (NULL);
> > +
> > printf_filtered (_("process %ld\n"), pid);
> > if (cmdline_f)
> > {
>
> [here and elsewhere, but here's a good example]
> It seems odd to have to continually set global state (the target fs)
> like this before being able to access the fs.
> Intuitively, I'd expect it to either be set once during some
> initialization phase, and/or have something passed as a parameter
> on down.
>
> Perhaps things will become clearer later in the patch set, but if
> you have reasons for doing it this way (that aren't mentioned
> elsewhere) it'd be good to hear them.

It was basically because the alternative was to add a parameter to
target_filesystem_is_local and target_fileio_{open,readlink,unlink}
(and all their target vector implementations) to pass around whatever
inferior you were talking about. You'd also have to make a lot more
changes to the remote protocol: either vFile:{open,unlink,readlink}
would need an extra argument (indicated with qSupported) or you'd need
new "fs" versions of each packet. Both Pedro and I thought that was
ugly.

Thanks,
Gary

--