[COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Pedro Alves]

This makes gdbserver share gdb's savestring, instead of baking its own.

Tested on x86_64 Fedora 17.

gdb/
2013-02-14  Pedro Alves  <palves@redhat.com>

	* utils.c (savestring): Don't #undef it.  Move function to
	common/common-utils.c.
	* common/common-utils.c: Include gdb_string.h.
	(savestring): Move here from utils.c.
	* common/common-utils.h (savestring): Declare.

gdb/gdbserver/
2013-02-14  Pedro Alves  <palves@redhat.com>

	* tracepoint.c (save_string): Delete.
	(add_tracepoint_action): Use savestring instead of save_string.
---
 gdb/common/common-utils.c  |   11 +++++++++++
 gdb/common/common-utils.h  |    6 ++++++
 gdb/gdbserver/tracepoint.c |   16 ++--------------
 gdb/utils.c                |   17 -----------------
 4 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index 60093a4..4204abf 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -24,6 +24,7 @@
 #endif
 
 #include "gdb_assert.h"
+#include "gdb_string.h"
 
 #include <stdlib.h>
 #include <stdio.h>
@@ -150,3 +151,13 @@ xsnprintf (char *str, size_t size, const char *format, ...)
 
   return ret;
 }
+
+char *
+savestring (const char *ptr, size_t len)
+{
+  char *p = (char *) xmalloc (len + 1);
+
+  memcpy (p, ptr, len);
+  p[len] = 0;
+  return p;
+}
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 2abc6d1..9b659d8 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -47,4 +47,10 @@ char *xstrvprintf (const char *format, va_list ap)
 int xsnprintf (char *str, size_t size, const char *format, ...)
      ATTRIBUTE_PRINTF (3, 4);
 
+/* Make a copy of the string at PTR with LEN characters
+   (and add a null character at the end in the copy).
+   Uses malloc to get the space.  Returns the address of the copy.  */
+
+char *savestring (const char *ptr, size_t len);
+
 #endif
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 51b6a0c..0ffedaa 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -1896,18 +1896,6 @@ find_next_tracepoint_by_number (struct tracepoint *prev_tp, int num)
 
 #endif
 
-static char *
-save_string (const char *str, size_t len)
-{
-  char *s;
-
-  s = xmalloc (len + 1);
-  memcpy (s, str, len);
-  s[len] = '\0';
-
-  return s;
-}
-
 /* Append another action to perform when the tracepoint triggers.  */
 
 static void
@@ -2028,7 +2016,7 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet)
 			 * tpoint->num_step_actions));
 	  tpoint->step_actions[tpoint->num_step_actions - 1] = action;
 	  tpoint->step_actions_str[tpoint->num_step_actions - 1]
-	    = save_string (act_start, act - act_start);
+	    = savestring (act_start, act - act_start);
 	}
       else
 	{
@@ -2041,7 +2029,7 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet)
 			sizeof (*tpoint->actions_str) * tpoint->numactions);
 	  tpoint->actions[tpoint->numactions - 1] = action;
 	  tpoint->actions_str[tpoint->numactions - 1]
-	    = save_string (act_start, act - act_start);
+	    = savestring (act_start, act - act_start);
 	}
     }
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index 408c6ce..eb99f4b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -89,9 +89,6 @@ extern PTR realloc ();		/* ARI: PTR */
 extern void free ();
 #endif
 
-/* readline defines this.  */
-#undef savestring
-
 void (*deprecated_error_begin_hook) (void);
 
 /* Prototypes for local functions */
@@ -1186,20 +1183,6 @@ myread (int desc, char *addr, int len)
   return orglen;
 }
 
-/* Make a copy of the string at PTR with LEN characters
-   (and add a null character at the end in the copy).
-   Uses malloc to get the space.  Returns the address of the copy.  */
-
-char *
-savestring (const char *ptr, size_t len)
-{
-  char *p = (char *) xmalloc (len + 1);
-
-  memcpy (p, ptr, len);
-  p[len] = 0;
-  return p;
-}
-
 void
 print_spaces (int n, struct ui_file *file)
 {

[COMMIT PATCH 1/2] savestring: Rename parameter 'size' to 'len'.

[Pedro Alves]

It's better to avoid needless confusion, and call a string's length,
length, instead of size, which is usually used to refer to sizeof of
the string (len+1):

	size_t len = strlen (str);
	size_t size = sizeof (str);

Tested on x86_64 Fedora 17.

2013-02-14  Pedro Alves  <palves@redhat.com>

	* utils.c (savestring): Rename parameter 'size' to 'len'.
---
 gdb/utils.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 282ab8b..408c6ce 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1186,17 +1186,17 @@ myread (int desc, char *addr, int len)
   return orglen;
 }
 
-/* Make a copy of the string at PTR with SIZE characters
+/* Make a copy of the string at PTR with LEN characters
    (and add a null character at the end in the copy).
    Uses malloc to get the space.  Returns the address of the copy.  */
 
 char *
-savestring (const char *ptr, size_t size)
+savestring (const char *ptr, size_t len)
 {
-  char *p = (char *) xmalloc (size + 1);
+  char *p = (char *) xmalloc (len + 1);
 
-  memcpy (p, ptr, size);
-  p[size] = 0;
+  memcpy (p, ptr, len);
+  p[len] = 0;
   return p;
 }
 

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> This makes gdbserver share gdb's savestring, instead of baking its own.

I'm tempted to change everything to use libiberty's xstrndup.

Tom

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Pedro Alves]

On 02/14/2013 06:07 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> This makes gdbserver share gdb's savestring, instead of baking its own.
> 
> I'm tempted to change everything to use libiberty's xstrndup.

gdbserver doesn't use libiberty though.

-- 
Pedro Alves

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Tom Tromey]

Tom> I'm tempted to change everything to use libiberty's xstrndup.

Pedro> gdbserver doesn't use libiberty though.

Yeah, I know.  It uses little bits of it though.  Why not all of it?
I guess to support cross builds of just gdbserver?

Tom

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Pedro Alves]

On 02/14/2013 06:09 PM, Tom Tromey wrote:
> Tom> I'm tempted to change everything to use libiberty's xstrndup.
> 
> Pedro> gdbserver doesn't use libiberty though.
> 
> Yeah, I know.  It uses little bits of it though.  Why not all of it?
> I guess to support cross builds of just gdbserver?

Yeah...  It'd be an easier sell if libiberty wasn't such a
kitchen sink.  I know that parts of libiberty would be problematic
for the Windows CE (I know, it's rotten by now), not sure about
other ports.

-- 
Pedro Alves

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Tom Tromey]

Pedro> Yeah...  It'd be an easier sell if libiberty wasn't such a
Pedro> kitchen sink.  I know that parts of libiberty would be problematic
Pedro> for the Windows CE (I know, it's rotten by now), not sure about
Pedro> other ports.

I suppose my main concern here is that the gdb/gdbserver unification
project will end up needing to reimplement chunks of libiberty.
But that would be a bad result, since libiberty already exists.

I don't really care so much about this particular function.
It is small, it doesn't really matter.  The xstrndup name is better
(IMO), but unimportantly so.

Tom

Re: [COMMIT PATCH 1/2] savestring: Rename parameter 'size' to 'len'.

[Mark Kettenis]

> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 Feb 2013 17:14:04 +0000
> 
> It's better to avoid needless confusion, and call a string's length,
> length, instead of size, which is usually used to refer to sizeof of
> the string (len+1):
> 
> 	size_t len = strlen (str);
> 	size_t size = sizeof (str);
> 
> Tested on x86_64 Fedora 17.

Agreed

> 2013-02-14  Pedro Alves  <palves@redhat.com>
> 
> 	* utils.c (savestring): Rename parameter 'size' to 'len'.
> ---
>  gdb/utils.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Pedro Alves]

On 02/14/2013 06:24 PM, Tom Tromey wrote:
> Pedro> Yeah...  It'd be an easier sell if libiberty wasn't such a
> Pedro> kitchen sink.  I know that parts of libiberty would be problematic
> Pedro> for the Windows CE (I know, it's rotten by now), not sure about
> Pedro> other ports.
> 
> I suppose my main concern here is that the gdb/gdbserver unification
> project will end up needing to reimplement chunks of libiberty.
> But that would be a bad result, since libiberty already exists.

Understood and agreed.  I just prefer to cross that bridge when we
come to it.  BTW, I think with ACX_CONFIGURE_DIR, the fact that libiberty
is at the top level and gdbserver isn't is no longer a problem.  That's
what we use to  configure the gnulib/ dir under gdbserver.

> I don't really care so much about this particular function.
> It is small, it doesn't really matter.  The xstrndup name is better
> (IMO), but unimportantly so.

-- 
Pedro Alves

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Joel Brobecker]

> Yeah...  It'd be an easier sell if libiberty wasn't such a
> kitchen sink.  I know that parts of libiberty would be problematic
> for the Windows CE (I know, it's rotten by now), not sure about
> other ports.

Lynx178 might also be an issue. Agreed on all other counts.
I was hoping that gnulib would be our saviour, but maybe it does not
implement everything we need (yet?).

-- 
Joel

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> Yeah...  It'd be an easier sell if libiberty wasn't such a
>> kitchen sink.  I know that parts of libiberty would be problematic
>> for the Windows CE (I know, it's rotten by now), not sure about
>> other ports.

Joel> Lynx178 might also be an issue. Agreed on all other counts.
Joel> I was hoping that gnulib would be our saviour, but maybe it does not
Joel> implement everything we need (yet?).

It has xstrndup, though it seems a bit weird to pull in 2 different
definitions of that function.  Also, gnulib updates seem tricky right
now due to the libiconv situation.

Tom

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Joel Brobecker]

> Joel> Lynx178 might also be an issue. Agreed on all other counts.
> Joel> I was hoping that gnulib would be our saviour, but maybe it does not
> Joel> implement everything we need (yet?).
> 
> It has xstrndup, though it seems a bit weird to pull in 2 different
> definitions of that function.  Also, gnulib updates seem tricky right
> now due to the libiconv situation.

Yeah, I was definitely thinking more long term... If the libiconv
situation is the only thing preventing us from doing an update,
I'd go ahead, because the only known platform where this is known
to potentially cause trouble is Lynx178, which I am going to guess
is a fairly little used system. And AFAICT, working around the issue
should be easily done by adding -DE...=... in CFLAGS when configuring
the debugger.

-- 
Joel

Re: [COMMIT PATCH 2/2] Move savestring to common/common-utils.c, make gdbserver use it.

[Tom Tromey]

Joel> Lynx178 might also be an issue. Agreed on all other counts.
Joel> I was hoping that gnulib would be our saviour, but maybe it does not
Joel> implement everything we need (yet?).

Tom> It has xstrndup, though it seems a bit weird to pull in 2 different
Tom> definitions of that function.  Also, gnulib updates seem tricky right
Tom> now due to the libiconv situation.

Joel> Yeah, I was definitely thinking more long term... If the libiconv
Joel> situation is the only thing preventing us from doing an update,
Joel> I'd go ahead, because the only known platform where this is known
Joel> to potentially cause trouble is Lynx178, which I am going to guess
Joel> is a fairly little used system. And AFAICT, working around the issue
Joel> should be easily done by adding -DE...=... in CFLAGS when configuring
Joel> the debugger.

Ok, in that case, after the branch I will resurrect my gnulib patch
series.  I'll update it and repost it when I do that.

Tom