[v5,1/8] Move utility functions to common/

Message ID 20140319223014.14668.89536.stgit@host1.jankratochvil.net
State Committed
Headers

Commit Message

Jan Kratochvil March 19, 2014, 10:30 p.m. UTC
  Hi,

some parts of the former patch have been reimplemented in the meantime by
other patches so this is only a part of the former cleanup.


Jan


gdb/
2014-02-26  Aleksandar Ristovski  <aristovski@qnx.com

	Move utility functions to common/.
	* cli/cli-utils.c (skip_spaces, skip_spaces_const): Move defs to
	common/common-utils.c.
	* cli/cli-utils.h (skip_spaces, skip_spaces_const): Move decls to
	common/common-utils.h.
	* common/common-utils.c (host-defs.h, ctype.h): Include.
	(HIGH_BYTE_POSN, is_digit_in_base, digit_to_int, strtoulst): Move
	from utils.c.
	(skip_spaces, skip_spaces_const): Move from cli/cli-utils.c.
	* common/host-defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Moved from
	defs.h.
	* common/common-utils.h (strtoulst): Move decl from utils.h.
	(hex2bin, bin2hex): Move decls from remote.h.
	(skip_spaces, skip_spaces_const): Move decls from cli/cli-utils.h.
	* defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Move to
	common/common-utils.h
	* utils.c (HIGH_BYTE_POSN, is_digit_in_base, digit_to_int)
	(strtoulst): Move to common/common-utils.c.
	* utils.h (strtoulst): Moved decl to common/common-utils.h.
---
 gdb/cli/cli-utils.c       |   24 ---------
 gdb/cli/cli-utils.h       |    9 ---
 gdb/common/common-utils.c |  125 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/common-utils.h |   11 ++++
 gdb/common/host-defs.h    |   21 ++++++++
 gdb/defs.h                |   19 -------
 gdb/utils.c               |   99 ------------------------------------
 gdb/utils.h               |    2 -
 8 files changed, 157 insertions(+), 153 deletions(-)
  

Comments

Sergio Durigan Junior March 24, 2014, 7:59 p.m. UTC | #1
On Wednesday, March 19 2014, Jan Kratochvil wrote:

> Hi,
>
> some parts of the former patch have been reimplemented in the meantime by
> other patches so this is only a part of the former cleanup.

Can't this go in independently?  I think they are nice cleanups.

Thanks,
  
Jan Kratochvil May 18, 2014, 7:05 p.m. UTC | #2
On Mon, 24 Mar 2014 20:59:48 +0100, Sergio Durigan Junior wrote:
> On Wednesday, March 19 2014, Jan Kratochvil wrote:
> > some parts of the former patch have been reimplemented in the meantime by
> > other patches so this is only a part of the former cleanup.
> 
> Can't this go in independently?  I think they are nice cleanups.

I do not think it is right without the later parts of the patchset - code
in gdb/common/ should be used by both gdb and gdbserver.  Otherwise the code
could remain in gdb/ only.


Jan
  
Tom Tromey May 19, 2014, 6:21 p.m. UTC | #3
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> some parts of the former patch have been reimplemented in the meantime by
Jan> other patches so this is only a part of the former cleanup.

Jan> 2014-02-26  Aleksandar Ristovski  <aristovski@qnx.com

Jan> 	Move utility functions to common/.
Jan> 	* cli/cli-utils.c (skip_spaces, skip_spaces_const): Move defs to
Jan> 	common/common-utils.c.
Jan> 	* cli/cli-utils.h (skip_spaces, skip_spaces_const): Move decls to
Jan> 	common/common-utils.h.
Jan> 	* common/common-utils.c (host-defs.h, ctype.h): Include.
Jan> 	(HIGH_BYTE_POSN, is_digit_in_base, digit_to_int, strtoulst): Move
Jan> 	from utils.c.
Jan> 	(skip_spaces, skip_spaces_const): Move from cli/cli-utils.c.
Jan> 	* common/host-defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Moved from
Jan> 	defs.h.
Jan> 	* common/common-utils.h (strtoulst): Move decl from utils.h.
Jan> 	(hex2bin, bin2hex): Move decls from remote.h.
Jan> 	(skip_spaces, skip_spaces_const): Move decls from cli/cli-utils.h.
Jan> 	* defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Move to
Jan> 	common/common-utils.h
Jan> 	* utils.c (HIGH_BYTE_POSN, is_digit_in_base, digit_to_int)
Jan> 	(strtoulst): Move to common/common-utils.c.
Jan> 	* utils.h (strtoulst): Moved decl to common/common-utils.h.

This is ok.

Tom
  
Tom Tromey May 19, 2014, 6:22 p.m. UTC | #4
Sergio> Can't this go in independently?  I think they are nice cleanups.

Jan> I do not think it is right without the later parts of the patchset
Jan> - code in gdb/common/ should be used by both gdb and gdbserver.
Jan> Otherwise the code could remain in gdb/ only.

If we had smaller files in common/ then we could put generically useful
things there and not care so much, relying on the linker to eliminate
bloat.

Tom
  
Doug Evans May 19, 2014, 10:45 p.m. UTC | #5
On Sun, May 18, 2014 at 12:05 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Mon, 24 Mar 2014 20:59:48 +0100, Sergio Durigan Junior wrote:
>> On Wednesday, March 19 2014, Jan Kratochvil wrote:
>> > some parts of the former patch have been reimplemented in the meantime by
>> > other patches so this is only a part of the former cleanup.
>>
>> Can't this go in independently?  I think they are nice cleanups.
>
> I do not think it is right without the later parts of the patchset - code
> in gdb/common/ should be used by both gdb and gdbserver.  Otherwise the code
> could remain in gdb/ only.

Hi.
I don't have a strong opinion on whether this particular patch can go
in right away, but I do have a comment.
I think we're building gdb wrong (where by "gdb" I mean
gdb+gdbserver+gdbreplay+...) if things can't go in common if they're
only used by gdb.
We should be building gdb out of useful building blocks, and it's just
easier to hack if said building blocks are already available, without
having to go through all the administrivia of moving them about or
what not.

If something like skip_spaces isn't used by gdbserver today, that
doesn't mean it won't be tomorrow.
IOW, I think of "common" as a collection of application-independent
routines, not dissimilar from libiberty, but for whatever reason
doesn't/can't live there instead. [btw, I'm not suggesting moving
routines from common to libiberty, but I'm guessing a quick glance
would find a few that should just as well live in libiberty - and be
usable by more people too]

Plus, I guess I have to ask, if down the road we find some routine in
common only used by a particular app, do we need to move it out of
common and into that app's directory?

My $0.02 anyways.
  
Doug Evans May 19, 2014, 10:48 p.m. UTC | #6
On Mon, May 19, 2014 at 11:22 AM, Tom Tromey <tromey@redhat.com> wrote:
> Sergio> Can't this go in independently?  I think they are nice cleanups.
>
> Jan> I do not think it is right without the later parts of the patchset
> Jan> - code in gdb/common/ should be used by both gdb and gdbserver.
> Jan> Otherwise the code could remain in gdb/ only.
>
> If we had smaller files in common/ then we could put generically useful
> things there and not care so much, relying on the linker to eliminate
> bloat.

How much bloat are we talking about?

Plus there's always -ffunction-sections -Wl,-gc-sections.
I've been meaning to try it, if only to see what more can be removed from gdb.
  

Patch

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index a0ebc11..49cdf83 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -213,30 +213,6 @@  number_is_in_list (char *list, int number)
 
 /* See documentation in cli-utils.h.  */
 
-char *
-skip_spaces (char *chp)
-{
-  if (chp == NULL)
-    return NULL;
-  while (*chp && isspace (*chp))
-    chp++;
-  return chp;
-}
-
-/* A const-correct version of the above.  */
-
-const char *
-skip_spaces_const (const char *chp)
-{
-  if (chp == NULL)
-    return NULL;
-  while (*chp && isspace (*chp))
-    chp++;
-  return chp;
-}
-
-/* See documentation in cli-utils.h.  */
-
 const char *
 skip_to_space_const (const char *chp)
 {
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index fed876b..c7f27e2 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -89,15 +89,6 @@  extern int get_number_or_range (struct get_number_or_range_state *state);
 
 extern int number_is_in_list (char *list, int number);
 
-/* Skip leading whitespace characters in INP, returning an updated
-   pointer.  If INP is NULL, return NULL.  */
-
-extern char *skip_spaces (char *inp);
-
-/* A const-correct version of the above.  */
-
-extern const char *skip_spaces_const (const char *inp);
-
 /* Skip leading non-whitespace characters in INP, returning an updated
    pointer.  If INP is NULL, return NULL.  */
 
diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index 29fe2c5..3e0c15e 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -23,11 +23,13 @@ 
 #include "defs.h"
 #endif
 
+#include "host-defs.h"
 #include "gdb_assert.h"
 #include <string.h>
 
 #include <stdlib.h>
 #include <stdio.h>
+#include <ctype.h>
 
 /* The xmalloc() (libiberty.h) family of memory management routines.
 
@@ -161,3 +163,126 @@  savestring (const char *ptr, size_t len)
   p[len] = 0;
   return p;
 }
+
+/* The bit offset of the highest byte in a ULONGEST, for overflow
+   checking.  */
+
+#define HIGH_BYTE_POSN ((sizeof (ULONGEST) - 1) * HOST_CHAR_BIT)
+
+/* True (non-zero) iff DIGIT is a valid digit in radix BASE,
+   where 2 <= BASE <= 36.  */
+
+static int
+is_digit_in_base (unsigned char digit, int base)
+{
+  if (!isalnum (digit))
+    return 0;
+  if (base <= 10)
+    return (isdigit (digit) && digit < base + '0');
+  else
+    return (isdigit (digit) || tolower (digit) < base - 10 + 'a');
+}
+
+static int
+digit_to_int (unsigned char c)
+{
+  if (isdigit (c))
+    return c - '0';
+  else
+    return tolower (c) - 'a' + 10;
+}
+
+/* As for strtoul, but for ULONGEST results.  */
+
+ULONGEST
+strtoulst (const char *num, const char **trailer, int base)
+{
+  unsigned int high_part;
+  ULONGEST result;
+  int minus = 0;
+  int i = 0;
+
+  /* Skip leading whitespace.  */
+  while (isspace (num[i]))
+    i++;
+
+  /* Handle prefixes.  */
+  if (num[i] == '+')
+    i++;
+  else if (num[i] == '-')
+    {
+      minus = 1;
+      i++;
+    }
+
+  if (base == 0 || base == 16)
+    {
+      if (num[i] == '0' && (num[i + 1] == 'x' || num[i + 1] == 'X'))
+	{
+	  i += 2;
+	  if (base == 0)
+	    base = 16;
+	}
+    }
+
+  if (base == 0 && num[i] == '0')
+    base = 8;
+
+  if (base == 0)
+    base = 10;
+
+  if (base < 2 || base > 36)
+    {
+      errno = EINVAL;
+      return 0;
+    }
+
+  result = high_part = 0;
+  for (; is_digit_in_base (num[i], base); i += 1)
+    {
+      result = result * base + digit_to_int (num[i]);
+      high_part = high_part * base + (unsigned int) (result >> HIGH_BYTE_POSN);
+      result &= ((ULONGEST) 1 << HIGH_BYTE_POSN) - 1;
+      if (high_part > 0xff)
+	{
+	  errno = ERANGE;
+	  result = ~ (ULONGEST) 0;
+	  high_part = 0;
+	  minus = 0;
+	  break;
+	}
+    }
+
+  if (trailer != NULL)
+    *trailer = &num[i];
+
+  result = result + ((ULONGEST) high_part << HIGH_BYTE_POSN);
+  if (minus)
+    return -result;
+  else
+    return result;
+}
+
+/* See documentation in cli-utils.h.  */
+
+char *
+skip_spaces (char *chp)
+{
+  if (chp == NULL)
+    return NULL;
+  while (*chp && isspace (*chp))
+    chp++;
+  return chp;
+}
+
+/* A const-correct version of the above.  */
+
+const char *
+skip_spaces_const (const char *chp)
+{
+  if (chp == NULL)
+    return NULL;
+  while (*chp && isspace (*chp))
+    chp++;
+  return chp;
+}
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 063698d..97889d5 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -71,4 +71,15 @@  int xsnprintf (char *str, size_t size, const char *format, ...)
 
 char *savestring (const char *ptr, size_t len);
 
+ULONGEST strtoulst (const char *num, const char **trailer, int base);
+
+/* Skip leading whitespace characters in INP, returning an updated
+   pointer.  If INP is NULL, return NULL.  */
+
+extern char *skip_spaces (char *inp);
+
+/* A const-correct version of the above.  */
+
+extern const char *skip_spaces_const (const char *inp);
+
 #endif
diff --git a/gdb/common/host-defs.h b/gdb/common/host-defs.h
index e4acef0..71a7029 100644
--- a/gdb/common/host-defs.h
+++ b/gdb/common/host-defs.h
@@ -19,6 +19,27 @@ 
 #ifndef HOST_DEFS_H
 #define HOST_DEFS_H
 
+#include <limits.h>
+
+/* Static host-system-dependent parameters for GDB.  */
+
+/* * Number of bits in a char or unsigned char for the target machine.
+   Just like CHAR_BIT in <limits.h> but describes the target machine.  */
+#if !defined (TARGET_CHAR_BIT)
+#define TARGET_CHAR_BIT 8
+#endif
+
+/* * If we picked up a copy of CHAR_BIT from a configuration file
+   (which may get it by including <limits.h>) then use it to set
+   the number of bits in a host char.  If not, use the same size
+   as the target.  */
+
+#if defined (CHAR_BIT)
+#define HOST_CHAR_BIT CHAR_BIT
+#else
+#define HOST_CHAR_BIT TARGET_CHAR_BIT
+#endif
+
 #ifdef __MSDOS__
 # define CANT_FORK
 # define GLOBAL_CURDIR
diff --git a/gdb/defs.h b/gdb/defs.h
index 47da43a..1aa7ad3 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -633,25 +633,6 @@  extern void *alloca ();
 
 enum { MAX_REGISTER_SIZE = 64 };
 
-/* Static target-system-dependent parameters for GDB.  */
-
-/* * Number of bits in a char or unsigned char for the target machine.
-   Just like CHAR_BIT in <limits.h> but describes the target machine.  */
-#if !defined (TARGET_CHAR_BIT)
-#define TARGET_CHAR_BIT 8
-#endif
-
-/* * If we picked up a copy of CHAR_BIT from a configuration file
-   (which may get it by including <limits.h>) then use it to set
-   the number of bits in a host char.  If not, use the same size
-   as the target.  */
-
-#if defined (CHAR_BIT)
-#define HOST_CHAR_BIT CHAR_BIT
-#else
-#define HOST_CHAR_BIT TARGET_CHAR_BIT
-#endif
-
 /* In findvar.c.  */
 
 extern LONGEST extract_signed_integer (const gdb_byte *, int,
diff --git a/gdb/utils.c b/gdb/utils.c
index 364470c..21186b8 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3026,105 +3026,6 @@  dummy_obstack_deallocate (void *object, void *data)
   return;
 }
 
-/* The bit offset of the highest byte in a ULONGEST, for overflow
-   checking.  */
-
-#define HIGH_BYTE_POSN ((sizeof (ULONGEST) - 1) * HOST_CHAR_BIT)
-
-/* True (non-zero) iff DIGIT is a valid digit in radix BASE,
-   where 2 <= BASE <= 36.  */
-
-static int
-is_digit_in_base (unsigned char digit, int base)
-{
-  if (!isalnum (digit))
-    return 0;
-  if (base <= 10)
-    return (isdigit (digit) && digit < base + '0');
-  else
-    return (isdigit (digit) || tolower (digit) < base - 10 + 'a');
-}
-
-static int
-digit_to_int (unsigned char c)
-{
-  if (isdigit (c))
-    return c - '0';
-  else
-    return tolower (c) - 'a' + 10;
-}
-
-/* As for strtoul, but for ULONGEST results.  */
-
-ULONGEST
-strtoulst (const char *num, const char **trailer, int base)
-{
-  unsigned int high_part;
-  ULONGEST result;
-  int minus = 0;
-  int i = 0;
-
-  /* Skip leading whitespace.  */
-  while (isspace (num[i]))
-    i++;
-
-  /* Handle prefixes.  */
-  if (num[i] == '+')
-    i++;
-  else if (num[i] == '-')
-    {
-      minus = 1;
-      i++;
-    }
-
-  if (base == 0 || base == 16)
-    {
-      if (num[i] == '0' && (num[i + 1] == 'x' || num[i + 1] == 'X'))
-	{
-	  i += 2;
-	  if (base == 0)
-	    base = 16;
-	}
-    }
-
-  if (base == 0 && num[i] == '0')
-    base = 8;
-
-  if (base == 0)
-    base = 10;
-
-  if (base < 2 || base > 36)
-    {
-      errno = EINVAL;
-      return 0;
-    }
-
-  result = high_part = 0;
-  for (; is_digit_in_base (num[i], base); i += 1)
-    {
-      result = result * base + digit_to_int (num[i]);
-      high_part = high_part * base + (unsigned int) (result >> HIGH_BYTE_POSN);
-      result &= ((ULONGEST) 1 << HIGH_BYTE_POSN) - 1;
-      if (high_part > 0xff)
-	{
-	  errno = ERANGE;
-	  result = ~ (ULONGEST) 0;
-	  high_part = 0;
-	  minus = 0;
-	  break;
-	}
-    }
-
-  if (trailer != NULL)
-    *trailer = &num[i];
-
-  result = result + ((ULONGEST) high_part << HIGH_BYTE_POSN);
-  if (minus)
-    return -result;
-  else
-    return result;
-}
-
 /* Simple, portable version of dirname that does not modify its
    argument.  */
 
diff --git a/gdb/utils.h b/gdb/utils.h
index d6df2ee..a949cd7 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -41,8 +41,6 @@  extern int streq (const char *, const char *);
 
 extern int subset_compare (char *, char *);
 
-ULONGEST strtoulst (const char *num, const char **trailer, int base);
-
 int compare_positive_ints (const void *ap, const void *bp);
 int compare_strings (const void *ap, const void *bp);