Re: [pushed][PATCH v3 1/4] Extended-remote follow exec

[Tom Tromey]

> "Tom" == Tom Tromey  writes:

> "Thomas" == Thomas Schwinge  writes:
Thomas> + struct cleanup *old_chain = make_cleanup (xfree, 
pathname);

Tom> Please don't add new cleanups to gdb.
Tom> We're in the process of removing them all.

Tom> Instead you can use gdb::unique_xmalloc_ptr, or std::string, or a
Tom> std::vector of some flavor.

I went ahead and added a patch to fix this to my series to remove
cleanups, so you don't need to do anything about this one.

Tom

Re: [pushed][PATCH v3 1/4] Extended-remote follow exec

[Tom Tromey]

> "Thomas" == Thomas Schwinge  writes:

Thomas> + struct cleanup *old_chain = make_cleanup (xfree, 
pathname);

Please don't add new cleanups to gdb.
We're in the process of removing them all.

Instead you can use gdb::unique_xmalloc_ptr, or std::string, or a
std::vector of some flavor.

thanks,
Tom

Re: [pushed][PATCH v3 1/4] Extended-remote follow exec

[Thomas Schwinge]

Hi!

On Fri, 17 Feb 2017 16:45:01 +, Pedro Alves  wrote:
> Only noticed this patch now.

Heh, and I've only now gotten back to completing this.  ;-)

> > On GNU/Hurd, there is no "#define PATH_MAX", so this fails to build.
> > (I'm aware that there is other PATH_MAX usage in GDB sources, which we
> > ought to fix at some point, for example in gdbserver -- which is not yet
> > enabled for GNU/Hurd.)
> > 
> > OK to push the following?  (Similar to Svante's patch in
> > .)
> 
> 
> > 
> > --- gdb/remote.c
> > +++ gdb/remote.c
> > @@ -6927,7 +6927,6 @@ Packet: '%s'\n"),
> >   else if (strprefix (p, p1, "exec"))
> > {
> >   ULONGEST ignored;
> > - char pathname[PATH_MAX];
> >   int pathlen;
> >  
> >   /* Determine the length of the execd pathname.  */
> > @@ -6936,11 +6935,12 @@ Packet: '%s'\n"),
> >  
> >   /* Save the pathname for event reporting and for
> >  the next run command.  */
> > + char *pathname = (char *) xmalloc (pathlen + 1);
> >   hex2bin (p1, (gdb_byte *) pathname, pathlen);
> >   pathname[pathlen] = '\0';
> 
> 
> hex2bin can throw, so wrap with a cleanup:
> 
>   char *pathname = (char *) xmalloc (pathlen + 1);
>   struct cleanup *old_chain = make_cleanup (xfree, pathname);
> hex2bin (p1, (gdb_byte *) pathname, pathlen);
> pathname[pathlen] = '\0';
>   discard_cleanups (old_chain);
> 
> OK with that change.

Thanks; pushed to master the attached commit
b671c7fb21306ce125717a44c30a71686bd24db1 "[gdb, hurd] Avoid using
'PATH_MAX' in 'gdb/remote.c'".


Grüße
 Thomas


>From b671c7fb21306ce125717a44c30a71686bd24db1 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Fri, 17 Feb 2017 16:45:01 +
Subject: [PATCH] [gdb, hurd] Avoid using 'PATH_MAX' in 'gdb/remote.c'

..., which is not defined in GNU/Hurd systems, and so commit
94585166dfea8232c248044f9f4b1c217dc4ac2e "Extended-remote follow-exec" caused:

[...]/gdb/remote.c: In member function 'void remote_target::remote_parse_stop_reply(const char*, stop_reply*)':
[...]/gdb/remote.c:7343:22: error: 'PATH_MAX' was not declared in this scope
char pathname[PATH_MAX];
  ^~~~

	gdb/
	* remote.c (remote_target::remote_parse_stop_reply): Avoid using
	'PATH_MAX'.
---
 gdb/ChangeLog | 6 ++
 gdb/remote.c  | 6 --
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f2bbd77558..bb27f74de1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-02-14  Thomas Schwinge  
+	Pedro Alves  
+
+	* remote.c (remote_target::remote_parse_stop_reply): Avoid using
+	'PATH_MAX'.
+
 2019-02-14  David Michael  
 	Samuel Thibault  
 	Thomas Schwinge  
diff --git a/gdb/remote.c b/gdb/remote.c
index 18e678d07a..85af01e4b7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7340,7 +7340,6 @@ Packet: '%s'\n"),
 	  else if (strprefix (p, p1, "exec"))
 	{
 	  ULONGEST ignored;
-	  char pathname[PATH_MAX];
 	  int pathlen;
 
 	  /* Determine the length of the execd pathname.  */
@@ -7349,11 +7348,14 @@ Packet: '%s'\n"),
 
 	  /* Save the pathname for event reporting and for
 		 the next run command.  */
+	  char *pathname = (char *) xmalloc (pathlen + 1);
+	  struct cleanup *old_chain = make_cleanup (xfree, pathname);
 	  hex2bin (p1, (gdb_byte *) pathname, pathlen);
 	  pathname[pathlen] = '\0';
+	  discard_cleanups (old_chain);
 
 	  /* This is freed during event handling.  */
-	  event->ws.value.execd_pathname = xstrdup (pathname);
+	  event->ws.value.execd_pathname = pathname;
 	  event->ws.kind = TARGET_WAITKIND_EXECD;
 
 	  /* Skip the registers included in this packet, since
-- 
2.19.2

Re: [pushed][PATCH v3 1/4] Extended-remote follow exec

[Pedro Alves]

Hi Thomas,

Only noticed this patch now.

> On GNU/Hurd, there is no "#define PATH_MAX", so this fails to build.
> (I'm aware that there is other PATH_MAX usage in GDB sources, which we
> ought to fix at some point, for example in gdbserver -- which is not yet
> enabled for GNU/Hurd.)
> 
> OK to push the following?  (Similar to Svante's patch in
> .)


> 
> --- gdb/remote.c
> +++ gdb/remote.c
> @@ -6927,7 +6927,6 @@ Packet: '%s'\n"),
> else if (strprefix (p, p1, "exec"))
>   {
> ULONGEST ignored;
> -   char pathname[PATH_MAX];
> int pathlen;
>  
> /* Determine the length of the execd pathname.  */
> @@ -6936,11 +6935,12 @@ Packet: '%s'\n"),
>  
> /* Save the pathname for event reporting and for
>the next run command.  */
> +   char *pathname = (char *) xmalloc (pathlen + 1);
> hex2bin (p1, (gdb_byte *) pathname, pathlen);
> pathname[pathlen] = '\0';


hex2bin can throw, so wrap with a cleanup:

  char *pathname = (char *) xmalloc (pathlen + 1);
  struct cleanup *old_chain = make_cleanup (xfree, pathname);
  hex2bin (p1, (gdb_byte *) pathname, pathlen);
  pathname[pathlen] = '\0';
  discard_cleanups (old_chain);

OK with that change.

Thanks,
Pedro Alves

Re: [pushed][PATCH v3 1/4] Extended-remote follow exec

[Thomas Schwinge]

Hi!

On Fri, 11 Sep 2015 11:38:15 -0700, Don Breazeal  wrote:
> Here is what I pushed.

> --- a/gdb/remote.c
> +++ b/gdb/remote.c

> @@ -6089,11 +6178,42 @@ Packet: '%s'\n"),
> event->ws.kind = TARGET_WAITKIND_VFORK_DONE;
> p = skip_to_semicolon (p1 + 1);
>   }
> +   else if (strncmp (p, "exec", p1 - p) == 0)
> + {
> +   ULONGEST ignored;
> +   char pathname[PATH_MAX];
> +   int pathlen;
> +
> +   /* Determine the length of the execd pathname.  */
> +   p = unpack_varlen_hex (++p1, );
> +   pathlen = (p - p1) / 2;
> +
> +   /* Save the pathname for event reporting and for
> +  the next run command.  */
> +   hex2bin (p1, (gdb_byte *) pathname, pathlen);
> +   pathname[pathlen] = '\0';
> +
> +   /* This is freed during event handling.  */
> +   event->ws.value.execd_pathname = xstrdup (pathname);
> +   event->ws.kind = TARGET_WAITKIND_EXECD;
> +
> +   /* Skip the registers included in this packet, since
> +  they may be for an architecture different from the
> +  one used by the original program.  */
> +   skipregs = 1;
> + }

On GNU/Hurd, there is no "#define PATH_MAX", so this fails to build.
(I'm aware that there is other PATH_MAX usage in GDB sources, which we
ought to fix at some point, for example in gdbserver -- which is not yet
enabled for GNU/Hurd.)

OK to push the following?  (Similar to Svante's patch in
.)

--- gdb/remote.c
+++ gdb/remote.c
@@ -6927,7 +6927,6 @@ Packet: '%s'\n"),
  else if (strprefix (p, p1, "exec"))
{
  ULONGEST ignored;
- char pathname[PATH_MAX];
  int pathlen;
 
  /* Determine the length of the execd pathname.  */
@@ -6936,11 +6935,12 @@ Packet: '%s'\n"),
 
  /* Save the pathname for event reporting and for
 the next run command.  */
+ char *pathname = (char *) xmalloc (pathlen + 1);
  hex2bin (p1, (gdb_byte *) pathname, pathlen);
  pathname[pathlen] = '\0';
 
  /* This is freed during event handling.  */
- event->ws.value.execd_pathname = xstrdup (pathname);
+ event->ws.value.execd_pathname = pathname;
  event->ws.kind = TARGET_WAITKIND_EXECD;
 
  /* Skip the registers included in this packet, since


Grüße
 Thomas