Patchwork [4/4] Introduce obstack_strndup

login
register
mail settings
Submitter Tom Tromey
Date Aug. 4, 2019, 2:51 a.m.
Message ID <20190804025110.15135-5-tom@tromey.com>
Download mbox | patch
Permalink /patch/33953/
State New
Headers show

Comments

Tom Tromey - Aug. 4, 2019, 2:51 a.m.
This introduces obstack_strndup and changes gdb to use it.

Note that obstack_strndup works like savestring, and not exactly like
xstrndup -- so perhaps it is misnamed, or perhaps it ought to be
changed.  The difference is that obstack_strndup uses the passed-in
length, while xstrndup uses strnlen to choose the length.

gdb/ChangeLog
2019-08-03  Tom Tromey  <tom@tromey.com>

	* stabsread.c (patch_block_stabs, read_one_struct_field)
	(read_enum_type): Use obstack_strndup.
	* rust-exp.y (rust_parser::copy_name): Use obstack_strndup.
	* gdb_obstack.h (obstack_strndup): Use obstack_strndup.
	* dwarf2read.c (guess_full_die_structure_name)
	(anonymous_struct_prefix): Use obstack_strndup.
	* dbxread.c (cp_set_block_scope): Use obstack_strndup.
	* c-exp.y (yylex): Use obstack_strndup.
	* ada-exp.y (write_object_renaming, write_ambiguous_var)
	(write_var_or_type): Use obstack_strndup.
---
 gdb/ChangeLog     | 13 +++++++++++++
 gdb/ada-exp.y     | 15 ++++++---------
 gdb/c-exp.y       |  6 +++---
 gdb/dbxread.c     |  3 +--
 gdb/dwarf2read.c  |  8 ++++----
 gdb/gdb_obstack.h |  9 +++++++++
 gdb/rust-exp.y    |  2 +-
 gdb/stabsread.c   |  8 ++++----
 8 files changed, 41 insertions(+), 23 deletions(-)
Kevin Buettner - Aug. 6, 2019, 9:28 p.m.
On Sat,  3 Aug 2019 20:51:10 -0600
Tom Tromey <tom@tromey.com> wrote:

> This introduces obstack_strndup and changes gdb to use it.
> 
> Note that obstack_strndup works like savestring, and not exactly like
> xstrndup -- so perhaps it is misnamed, or perhaps it ought to be
> changed.  The difference is that obstack_strndup uses the passed-in
> length, while xstrndup uses strnlen to choose the length.
> 
> gdb/ChangeLog
> 2019-08-03  Tom Tromey  <tom@tromey.com>
> 
> 	* stabsread.c (patch_block_stabs, read_one_struct_field)
> 	(read_enum_type): Use obstack_strndup.
> 	* rust-exp.y (rust_parser::copy_name): Use obstack_strndup.
> 	* gdb_obstack.h (obstack_strndup): Use obstack_strndup.
> 	* dwarf2read.c (guess_full_die_structure_name)
> 	(anonymous_struct_prefix): Use obstack_strndup.
> 	* dbxread.c (cp_set_block_scope): Use obstack_strndup.
> 	* c-exp.y (yylex): Use obstack_strndup.
> 	* ada-exp.y (write_object_renaming, write_ambiguous_var)
> 	(write_var_or_type): Use obstack_strndup.

I think the name (of obstack_strndup) is okay, though I do think that
it might be worth noting that exactly N characters are duplicated in
obstack_strndup's comment.  I.e, it won't be shorter if the string
being copied happens to be shorter.

It took me a while to realize that the "0" in obstack_copy0 /
obstack_grow0 cause a '\0' character to be tacked onto the end.

Anyway, aside from perhaps updating the comment for obstack_strndup, it
looks good to me.

Kevin
Tom Tromey - Aug. 7, 2019, 2:04 a.m.
Kevin> I think the name (of obstack_strndup) is okay, though I do think that
Kevin> it might be worth noting that exactly N characters are duplicated in
Kevin> obstack_strndup's comment.  I.e, it won't be shorter if the string
Kevin> being copied happens to be shorter.

Kevin> It took me a while to realize that the "0" in obstack_copy0 /
Kevin> obstack_grow0 cause a '\0' character to be tacked onto the end.

Kevin> Anyway, aside from perhaps updating the comment for obstack_strndup, it
Kevin> looks good to me.

Thanks.  I've made this change & the other one you pointed out, and I'm
checking it in.

Tom

Patch

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index 2c8f3d6572e..f36aabaf150 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -812,8 +812,8 @@  write_object_renaming (struct parser_state *par_state,
   if (orig_left_context == NULL)
     orig_left_context = get_selected_block (NULL);
 
-  name = (char *) obstack_copy0 (&temp_parse_space, renamed_entity,
-				 renamed_entity_len);
+  name = obstack_strndup (&temp_parse_space, renamed_entity,
+			  renamed_entity_len);
   ada_lookup_encoded_symbol (name, orig_left_context, VAR_DOMAIN, &sym_info);
   if (sym_info.symbol == NULL)
     error (_("Could not find renamed variable: %s"), ada_decode (name));
@@ -881,9 +881,8 @@  write_object_renaming (struct parser_state *par_state,
 	    if (end == NULL)
 	      end = renaming_expr + strlen (renaming_expr);
 
-	    index_name
-	      = (char *) obstack_copy0 (&temp_parse_space, renaming_expr,
-					end - renaming_expr);
+	    index_name = obstack_strndup (&temp_parse_space, renaming_expr,
+					  end - renaming_expr);
 	    renaming_expr = end;
 
 	    ada_lookup_encoded_symbol (index_name, orig_left_context,
@@ -1107,8 +1106,7 @@  write_ambiguous_var (struct parser_state *par_state,
 
   memset (sym, 0, sizeof (struct symbol));
   SYMBOL_DOMAIN (sym) = UNDEF_DOMAIN;
-  SYMBOL_LINKAGE_NAME (sym)
-    = (const char *) obstack_copy0 (&temp_parse_space, name, len);
+  SYMBOL_LINKAGE_NAME (sym) = obstack_strndup (&temp_parse_space, name, len);
   SYMBOL_LANGUAGE (sym) = language_ada;
 
   write_exp_elt_opcode (par_state, OP_VAR_VALUE);
@@ -1206,8 +1204,7 @@  write_var_or_type (struct parser_state *par_state,
 
   encoded_name = ada_encode (name0.ptr);
   name_len = strlen (encoded_name);
-  encoded_name
-    = (char *) obstack_copy0 (&temp_parse_space, encoded_name, name_len);
+  encoded_name = obstack_strndup (&temp_parse_space, encoded_name, name_len);
   for (depth = 0; depth < MAX_RENAMING_CHAIN_LENGTH; depth += 1)
     {
       int tail_index;
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 439a72a10de..e0abbd83ddb 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -3291,9 +3291,9 @@  yylex (void)
   if (checkpoint > 0)
     {
       current.value.sval.ptr
-	= (const char *) obstack_copy0 (&cpstate->expansion_obstack,
-					current.value.sval.ptr,
-					current.value.sval.length);
+	= obstack_strndup (&cpstate->expansion_obstack,
+			   current.value.sval.ptr,
+			   current.value.sval.length);
 
       token_fifo[0] = current;
       if (checkpoint > 1)
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 0b370cd2e33..e339d1f7ecf 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -2354,8 +2354,7 @@  cp_set_block_scope (const struct symbol *symbol,
       const char *name = SYMBOL_DEMANGLED_NAME (symbol);
       unsigned int prefix_len = cp_entire_prefix_len (name);
 
-      block_set_scope (block,
-		       (const char *) obstack_copy0 (obstack, name, prefix_len),
+      block_set_scope (block, obstack_strndup (obstack, name, prefix_len),
 		       obstack);
     }
 }
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0f7ca54715b..a3b2ab0f289 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -22351,7 +22351,7 @@  guess_full_die_structure_name (struct die_info *die, struct dwarf2_cu *cu)
 		      if (actual_name_len > die_name_len + 2
 			  && actual_name[actual_name_len
 					 - die_name_len - 1] == ':')
-			name = (char *) obstack_copy0 (
+			name = obstack_strndup (
 			  &objfile->per_bfd->storage_obstack,
 			  actual_name, actual_name_len - die_name_len - 2);
 		    }
@@ -22395,9 +22395,9 @@  anonymous_struct_prefix (struct die_info *die, struct dwarf2_cu *cu)
     return "";
 
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
-  return (char *) obstack_copy0 (&objfile->per_bfd->storage_obstack,
-				 DW_STRING (attr),
-				 &base[-1] - DW_STRING (attr));
+  return obstack_strndup (&objfile->per_bfd->storage_obstack,
+			  DW_STRING (attr),
+			  &base[-1] - DW_STRING (attr));
 }
 
 /* Return the name of the namespace/class that DIE is defined within,
diff --git a/gdb/gdb_obstack.h b/gdb/gdb_obstack.h
index db849a06444..1e831ae54d1 100644
--- a/gdb/gdb_obstack.h
+++ b/gdb/gdb_obstack.h
@@ -105,6 +105,15 @@  obstack_strdup (struct obstack *obstackp, const std::string &string)
 				 string.size ());
 }
 
+/* Duplicate the first N characters of STRING, returning a
+   \0-terminated string that's allocated on the obstack OBSTACKP.  */
+
+static inline char *
+obstack_strndup (struct obstack *obstackp, const char *string, size_t n)
+{
+  return (char *) obstack_copy0 (obstackp, string, n);
+}
+
 /* An obstack that frees itself on scope exit.  */
 struct auto_obstack : obstack
 {
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index bf6566b8d29..60ce2cde057 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -1011,7 +1011,7 @@  static const struct token_info operator_tokens[] =
 const char *
 rust_parser::copy_name (const char *name, int len)
 {
-  return (const char *) obstack_copy0 (&obstack, name, len);
+  return obstack_strndup (&obstack, name, len);
 }
 
 /* Helper function to make an stoken from a C string.  */
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 23da5f94ebb..48c88ded553 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -426,8 +426,8 @@  patch_block_stabs (struct pending *symbols, struct pending_stabs *stabs,
 	      SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
 	      SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
 	      SYMBOL_SET_LINKAGE_NAME
-		(sym, (char *) obstack_copy0 (&objfile->objfile_obstack,
-					      name, pp - name));
+		(sym, obstack_strndup (&objfile->objfile_obstack,
+				       name, pp - name));
 	      pp += 2;
 	      if (*(pp - 1) == 'F' || *(pp - 1) == 'f')
 		{
@@ -2841,7 +2841,7 @@  read_one_struct_field (struct stab_field_info *fip, const char **pp,
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
 
   fip->list->field.name
-    = (const char *) obstack_copy0 (&objfile->objfile_obstack, *pp, p - *pp);
+    = obstack_strndup (&objfile->objfile_obstack, *pp, p - *pp);
   *pp = p + 1;
 
   /* This means we have a visibility for a field coming.  */
@@ -3641,7 +3641,7 @@  read_enum_type (const char **pp, struct type *type,
       p = *pp;
       while (*p != ':')
 	p++;
-      name = (char *) obstack_copy0 (&objfile->objfile_obstack, *pp, p - *pp);
+      name = obstack_strndup (&objfile->objfile_obstack, *pp, p - *pp);
       *pp = p + 1;
       n = read_huge_number (pp, ',', &nbits, 0);
       if (nbits != 0)