Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver

[Gary Benson <gbenson at redhat dot com>]

Doug Evans wrote:
> Gary Benson <gbenson@redhat.com> writes:
> > diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> > index 126c861..dc7802d 100644
> > --- a/gdb/gdbserver/target.h
> > +++ b/gdb/gdbserver/target.h
> > @@ -394,6 +394,14 @@ struct target_ops
> >  
> >    /* Return true if target supports range stepping.  */
> >    int (*supports_range_stepping) (void);
> > +
> > +  /* Return the pathname of the executable file that was run to
> > +     create the process PID.  If the executable file cannot be
> > +     determined, NULL is returned.  Otherwise, a pointer to a
> > +     character string containing the pathname is returned.  This
> > +     string should be copied into a buffer by the client if the
> > +     string will not be immediately used, or if it must persist.  */
> > +  char *(*pid_to_exec_file) (int pid);
> >  };
> >  
> >  extern struct target_ops *the_target;
> 
> IWBN to have some clarity on what the pathname result can and cannot be.
> 
> Perhaps nitpicky, but the less ambiguity the better.  I think(!) the
> intent is that the path name is the full path name, but I could be
> wrong.
> 
> Another issue is whether the path has been real-path'd.  [all
> symlinks resolved] I don't have a strong opinion on that, but I do
> think we should at least require full paths to be returned here.

I will use the "the full absolute name of the file" as Eli suggested
for the documentation.  I will also change the gdb/target.h version
to match.

It's not necessary for all symlinks to be resolved.  I think the
intention is for the function to return what the operating system
says the executable's filename is, but of course non of these old
parts of GDB are documented to that extent so I'm guessing.

> const char * result?

Not const for the same reason as the GDB side.

> Also, I was going to say we need to pick a type for "pid" and
> consistently use it, but that's a whole 'nother discussion, and
> this patch set needn't be bogged down by it.

Quite :)

Thanks,
Gary

-- 
http://gbenson.net/