Re: [PATCH 2/7] Introduce target_fileio_set_fs

Doug Evans
2015-04-17T03:04:58Z

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.
>
> gdb/ChangeLog:
>
> * target.h (struct inferior): New forward declaration.
> (struct target_ops) <to_fileio_set_fs>: New field.
> (target_fileio_set_fs): New declaration.
> * target-delegates.c: Regenerate.
> * target.c (target_fileio_set_fs): New function.
> * exec.c (exec_file_attach): Call target_fileio_set_fs.
> * gdb_bfd.c (inferior.h): New include.
> (gdb_bfd_iovec_fileio_open): Rename argument. Call
> target_fileio_set_fs.
> (gdb_bfd_open): Call target_fileio_set_fs. Pass current
> inferior as argument to gdb_bfd_iovec_fileio_open.
> * linux-tdep.c (linux_info_proc): Call target_fileio_set_fs.
> (linux_find_memory_regions_full): Likewise.
> (linux_fill_prpsinfo): Likewise.
> * solib.c (solib_find_1): Likewise.
> ...
> 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 (struct gdbarch *gdbarch, const char *args,
> 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.