[v5,1/8] Move utility functions to common/
Commit Message
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
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,
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
>>>>> "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
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
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.
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.
@@ -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)
{
@@ -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. */
@@ -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;
+}
@@ -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
@@ -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
@@ -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,
@@ -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. */
@@ -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);