[PATCH] Use exec_file_find to prepend gdb_sysroot in follow_exec

[PATCH] Use exec_file_find to prepend gdb_sysroot in follow_exec

[Gary Benson]

Hi all,

This commit updates follow_exec to use exec_file_find to prefix
the new executable's filename with gdb_sysroot rather than doing
it longhand.

Built and regtested on RHEL6.6 x86_64.

Ok to commit?

Cheers,
Gary


gdb/ChangeLog:

	* infrun.c (solist.h): New include.
	(follow_exec): Use exec_file_find to prefix execd_pathname
	with gdb_sysroot.
---
 gdb/ChangeLog |    6 ++++++
 gdb/infrun.c  |   18 +++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7870f70..f09e2da 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -60,6 +60,7 @@
 #include "target-descriptions.h"
 #include "target-dcache.h"
 #include "terminal.h"
+#include "solist.h"
 
 /* Prototypes for local functions */
 
@@ -1133,15 +1134,18 @@ follow_exec (ptid_t ptid, char *execd_pathname)
 
   breakpoint_init_inferior (inf_execd);
 
-  if (gdb_sysroot && *gdb_sysroot)
+  if (gdb_sysroot != NULL && *gdb_sysroot != '\0')
     {
-      char *name = alloca (strlen (gdb_sysroot)
-			    + strlen (execd_pathname)
-			    + 1);
+      int fd = -1;
+      char *name;
 
-      strcpy (name, gdb_sysroot);
-      strcat (name, execd_pathname);
-      execd_pathname = name;
+      name = exec_file_find (execd_pathname, &fd);
+      if (fd >= 0)
+	close (fd);
+
+      execd_pathname = alloca (strlen (name) + 1);
+      strcpy (execd_pathname, name);
+      xfree (name);
     }
 
   /* Reset the shared library package.  This ensures that we get a
-- 
1.7.1

[PING][PATCH] Use exec_file_find to prepend gdb_sysroot in follow_exec

[Gary Benson]

Ping:
https://sourceware.org/ml/gdb-patches/2015-04/msg00674.html

Gary Benson wrote:
> Hi all,
> 
> This commit updates follow_exec to use exec_file_find to prefix
> the new executable's filename with gdb_sysroot rather than doing
> it longhand.
> 
> Built and regtested on RHEL6.6 x86_64.
> 
> Ok to commit?
> 
> Cheers,
> Gary
> 
> 
> gdb/ChangeLog:
> 
> 	* infrun.c (solist.h): New include.
> 	(follow_exec): Use exec_file_find to prefix execd_pathname
> 	with gdb_sysroot.
> ---
>  gdb/ChangeLog |    6 ++++++
>  gdb/infrun.c  |   18 +++++++++++-------
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 7870f70..f09e2da 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -60,6 +60,7 @@
>  #include "target-descriptions.h"
>  #include "target-dcache.h"
>  #include "terminal.h"
> +#include "solist.h"
>  
>  /* Prototypes for local functions */
>  
> @@ -1133,15 +1134,18 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>  
>    breakpoint_init_inferior (inf_execd);
>  
> -  if (gdb_sysroot && *gdb_sysroot)
> +  if (gdb_sysroot != NULL && *gdb_sysroot != '\0')
>      {
> -      char *name = alloca (strlen (gdb_sysroot)
> -			    + strlen (execd_pathname)
> -			    + 1);
> +      int fd = -1;
> +      char *name;
>  
> -      strcpy (name, gdb_sysroot);
> -      strcat (name, execd_pathname);
> -      execd_pathname = name;
> +      name = exec_file_find (execd_pathname, &fd);
> +      if (fd >= 0)
> +	close (fd);
> +
> +      execd_pathname = alloca (strlen (name) + 1);
> +      strcpy (execd_pathname, name);
> +      xfree (name);
>      }
>  
>    /* Reset the shared library package.  This ensures that we get a
> -- 
> 1.7.1
> 

-- 
http://gbenson.net/

Re: [PATCH] Use exec_file_find to prepend gdb_sysroot in follow_exec

[Pedro Alves]

On 04/17/2015 02:28 PM, Gary Benson wrote:
> Hi all,
> 
> This commit updates follow_exec to use exec_file_find to prefix
> the new executable's filename with gdb_sysroot rather than doing
> it longhand.
> 
> Built and regtested on RHEL6.6 x86_64.
> 
> Ok to commit?

OK.

> -  if (gdb_sysroot && *gdb_sysroot)
> +  if (gdb_sysroot != NULL && *gdb_sysroot != '\0')
>      {
> -      char *name = alloca (strlen (gdb_sysroot)
> -			    + strlen (execd_pathname)
> -			    + 1);
> +      int fd = -1;
> +      char *name;
>  
> -      strcpy (name, gdb_sysroot);
> -      strcat (name, execd_pathname);
> -      execd_pathname = name;
> +      name = exec_file_find (execd_pathname, &fd);
> +      if (fd >= 0)
> +	close (fd);

We now have at least two places that need to remember to call
close.  IWBN if we hid that close in a exec_file_find variant, so
that callers didn't have to recall to do it.  Maybe
rename exec_file_find to (e.g.) exec_file_find_fd and reuse
the exec_file_find name, even.

Thanks,
Pedro Alves

Re: [PATCH] Use exec_file_find to prepend gdb_sysroot in follow_exec

[Gary Benson]

Pedro Alves wrote:
> On 04/17/2015 02:28 PM, Gary Benson wrote:
> > This commit updates follow_exec to use exec_file_find to prefix
> > the new executable's filename with gdb_sysroot rather than doing
> > it longhand.
> > 
> > Built and regtested on RHEL6.6 x86_64.
> > 
> > Ok to commit?
> 
> OK.

Thanks, I pushed it.

> > -  if (gdb_sysroot && *gdb_sysroot)
> > +  if (gdb_sysroot != NULL && *gdb_sysroot != '\0')
> >      {
> > -      char *name = alloca (strlen (gdb_sysroot)
> > -			    + strlen (execd_pathname)
> > -			    + 1);
> > +      int fd = -1;
> > +      char *name;
> >  
> > -      strcpy (name, gdb_sysroot);
> > -      strcat (name, execd_pathname);
> > -      execd_pathname = name;
> > +      name = exec_file_find (execd_pathname, &fd);
> > +      if (fd >= 0)
> > +	close (fd);
> 
> We now have at least two places that need to remember to call close.
> IWBN if we hid that close in a exec_file_find variant, so that
> callers didn't have to recall to do it.  Maybe rename exec_file_find
> to (e.g.) exec_file_find_fd and reuse the exec_file_find name, even.

Good shout.  How about I fix solib_find_1 to accept fd == NULL, and
put the closing logic in there?  That would work for solib_find too
then (I'm not sure if there are any "solib_find; close" places in
GDB but I'll look.)

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH] Use exec_file_find to prepend gdb_sysroot in follow_exec

[Pedro Alves]

On 04/28/2015 12:29 PM, Gary Benson wrote:
> Pedro Alves wrote:

>> We now have at least two places that need to remember to call close.
>> IWBN if we hid that close in a exec_file_find variant, so that
>> callers didn't have to recall to do it.  Maybe rename exec_file_find
>> to (e.g.) exec_file_find_fd and reuse the exec_file_find name, even.
> 
> Good shout.  How about I fix solib_find_1 to accept fd == NULL, and
> put the closing logic in there?  That

Sounds good.

> would work for solib_find too then (I'm not sure if there are
> any "solib_find; close" places in GDB but I'll look.)

Thanks,
Pedro Alves

[PATCH] Allow passing fd == NULL to exec_file_find and solib_find

[Gary Benson]

Hi all,

This commit allows NULL to be passed as the int *fd argument
to exec_file_find and solib_find to simplify use cases where
the caller does not require the file to be opened.

Built and regtested on RHEL6.6 x86_64.

Ok to commit?

Cheers,
Gary

gdb/ChangeLog:

	* solib.c (solib_find_1): Allow fd argument to be NULL.
	(exec_file_find): Update comment.
	(solib_find): Likewise.
	* exec.c (exec_file_locate_attach): Use NULL as fd
	argument to exec_file_find to avoid having to close
	the opened file.
	* infrun.c (follow_exec): Likewise.
---
 gdb/ChangeLog |   10 ++++++++++
 gdb/exec.c    |    8 +-------
 gdb/infrun.c  |    7 +------
 gdb/solib.c   |   25 +++++++++++++++++--------
 4 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index 872b86c..8a4ab6f 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -156,13 +156,7 @@ exec_file_locate_attach (int pid, int from_tty)
      is absolute then prefix the filename with gdb_sysroot.  */
   if (gdb_sysroot != NULL && *gdb_sysroot != '\0'
       && IS_ABSOLUTE_PATH (exec_file))
-    {
-      int fd = -1;
-
-      full_exec_path = exec_file_find (exec_file, &fd);
-      if (fd >= 0)
-	close (fd);
-    }
+    full_exec_path = exec_file_find (exec_file, NULL);
 
   if (full_exec_path == NULL)
     {
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f09e2da..a4f0b9f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1136,12 +1136,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
 
   if (gdb_sysroot != NULL && *gdb_sysroot != '\0')
     {
-      int fd = -1;
-      char *name;
-
-      name = exec_file_find (execd_pathname, &fd);
-      if (fd >= 0)
-	close (fd);
+      char *name = exec_file_find (execd_pathname, NULL);
 
       execd_pathname = alloca (strlen (name) + 1);
       strcpy (execd_pathname, name);
diff --git a/gdb/solib.c b/gdb/solib.c
index 2466235..358a0a2 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -114,8 +114,9 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 
 /* Return the full pathname of a binary file (the main executable
    or a shared library file), or NULL if not found.  The returned
-   pathname is malloc'ed and must be freed by the caller.  *FD is
-   set to either -1 or an open file handle for the binary file.
+   pathname is malloc'ed and must be freed by the caller.  If FD
+   is non-NULL, *FD is set to either -1 or an open file handle for
+   the binary file.
 
    Global variable GDB_SYSROOT is used as a prefix directory
    to search for binary files if they have an absolute path.
@@ -254,7 +255,8 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
   /* Handle files to be accessed via the target.  */
   if (is_target_filename (temp_pathname))
     {
-      *fd = -1;
+      if (fd != NULL)
+	*fd = -1;
       do_cleanups (old_chain);
       return temp_pathname;
     }
@@ -367,14 +369,21 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
 			OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, in_pathname,
 			O_RDONLY | O_BINARY, &temp_pathname);
 
-  *fd = found_file;
+  if (fd == NULL)
+    {
+      if (found_file >= 0)
+	close (found_file);
+    }
+  else
+    *fd = found_file;
+
   return temp_pathname;
 }
 
 /* Return the full pathname of the main executable, or NULL if not
    found.  The returned pathname is malloc'ed and must be freed by
-   the caller.  *FD is set to either -1 or an open file handle for
-   the main executable.
+   the caller.  If FD is non-NULL, *FD is set to either -1 or an open
+   file handle for the main executable.
 
    The search algorithm used is described in solib_find_1's comment
    above.  */
@@ -405,8 +414,8 @@ exec_file_find (char *in_pathname, int *fd)
 
 /* Return the full pathname of a shared library file, or NULL if not
    found.  The returned pathname is malloc'ed and must be freed by
-   the caller.  *FD is set to either -1 or an open file handle for
-   the shared library.
+   the caller.  If FD is non-NULL, *FD is set to either -1 or an open
+   file handle for the shared library.
 
    The search algorithm used is described in solib_find_1's comment
    above.  */
-- 
1.7.1

Re: [PATCH] Allow passing fd == NULL to exec_file_find and solib_find

[Pedro Alves]

On 04/28/2015 07:18 PM, Gary Benson wrote:
> Hi all,
> 
> This commit allows NULL to be passed as the int *fd argument
> to exec_file_find and solib_find to simplify use cases where
> the caller does not require the file to be opened.
> 
> Built and regtested on RHEL6.6 x86_64.
> 
> Ok to commit?
> 

OK.

Thanks,
Pedro Alves

Re: [PATCH] Allow passing fd == NULL to exec_file_find and solib_find

[Gary Benson]

Pedro Alves wrote:
> On 04/28/2015 07:18 PM, Gary Benson wrote:
> > This commit allows NULL to be passed as the int *fd argument
> > to exec_file_find and solib_find to simplify use cases where
> > the caller does not require the file to be opened.
> > 
> > Built and regtested on RHEL6.6 x86_64.
> > 
> > Ok to commit?
> 
> OK.

Thanks, pushed.

Cheers,
Gary

-- 
http://gbenson.net/