[PATCH] use xstrdup and concat more

[PATCH] use xstrdup and concat more

[tbsaunde+binutils]

From: Trevor Saunders <tbsaunde+binutils@tbsaunde.org>

Hi,

$subject, the diffstat should speak for it self :)

built crosses to one target per tc-*.c, and regtested x86_64-linux-gnu, ok?

Trev

gas/ChangeLog:

2016-04-24  Trevor Saunders  <tbsaunde+binutils@tbsaunde.org>

	* config/obj-coff.c (obj_coff_def): Simplify string copying.
	(weak_name2altname): Likewise.
	(weak_uniquify): Likewise.
	(obj_coff_section): Likewise.
	(obj_coff_init_stab_section): Likewise.
	* config/obj-elf.c (obj_elf_section_name): Likewise.
	(obj_elf_init_stab_section): Likewise.
	* config/obj-evax.c (evax_shorten_name): Likewise.
	* config/obj-macho.c (obj_mach_o_make_or_get_sect): Likewise.
	* config/tc-aarch64.c (create_register_alias): Likewise.
	* config/tc-alpha.c (load_expression): Likewise.
	(s_alpha_file): Likewise.
	(s_alpha_section_name): Likewise.
	(tc_gen_reloc): Likewise.
	* config/tc-arc.c (md_assemble): Likewise.
	* config/tc-arm.c (create_neon_reg_alias): Likewise.
	(start_unwind_section): Likewise.
	* config/tc-hppa.c (pa_build_unwind_subspace): Likewise.
	(hppa_elf_mark_end_of_function): Likewise.
	* config/tc-nios2.c (nios2_modify_arg): Likewise.
	(nios2_negate_arg): Likewise.
	* config/tc-rx.c (rx_section): Likewise.
	* config/tc-sh64.c (sh64_consume_datalabel): Likewise.
	* config/tc-tic30.c (tic30_find_parallel_insn): Likewise.
	* config/tc-tic54x.c (tic54x_include): Likewise.
	(tic54x_macro_info): Likewise.
	(subsym_get_arg): Likewise.
	(subsym_substitute): Likewise.
	(tic54x_start_line_hook): Likewise.
	* config/tc-xtensa.c (xtensa_literal_prefix): Likewise.
	(xg_reverse_shift_count): Likewise.
	* config/xtensa-relax.c (enter_opname_n): Likewise.
	(split_string): Likewise.
	* dwarf2dbg.c (get_filenum): Likewise.
	(process_entries): Likewise.
	* expr.c (operand): Likewise.
	* itbl-ops.c (alloc_entry): Likewise.
	* listing.c (listing_message): Likewise.
	(listing_title): Likewise.
	* macro.c (check_macro): Likewise.
	* stabs.c (s_xstab): Likewise.
	* symbols.c (symbol_relc_make_expr): Likewise.
	* write.c (compress_debug): Likewise.
---
 gas/config/obj-coff.c     | 24 ++++------------
 gas/config/obj-elf.c      |  8 ++----
 gas/config/obj-evax.c     |  7 ++---
 gas/config/obj-macho.c    | 11 +-------
 gas/config/tc-aarch64.c   |  4 +--
 gas/config/tc-alpha.c     | 17 +++---------
 gas/config/tc-arc.c       |  4 +--
 gas/config/tc-arm.c       | 19 ++-----------
 gas/config/tc-hppa.c      | 71 +++++++++++++++++++----------------------------
 gas/config/tc-nios2.c     | 13 ++-------
 gas/config/tc-rx.c        |  5 +---
 gas/config/tc-sh64.c      |  5 +---
 gas/config/tc-tic30.c     |  7 ++---
 gas/config/tc-tic54x.c    | 24 +++++-----------
 gas/config/tc-xtensa.c    |  8 ++----
 gas/config/xtensa-relax.c |  8 ++----
 gas/dwarf2dbg.c           | 10 ++-----
 gas/expr.c                | 14 +++-------
 gas/itbl-ops.c            |  4 +--
 gas/listing.c             |  9 ++----
 gas/macro.c               |  4 +--
 gas/stabs.c               |  4 +--
 gas/symbols.c             | 27 ++++++------------
 gas/write.c               |  5 +---
 24 files changed, 86 insertions(+), 226 deletions(-)

diff --git a/gas/config/obj-coff.c b/gas/config/obj-coff.c
index 03be655..02c0634 100644
--- a/gas/config/obj-coff.c
+++ b/gas/config/obj-coff.c
@@ -592,7 +592,6 @@ obj_coff_def (int what ATTRIBUTE_UNUSED)
   char name_end;		/* Char after the end of name.  */
   char *symbol_name;		/* Name of the debug symbol.  */
   char *symbol_name_copy;	/* Temporary copy of the name.  */
-  unsigned int symbol_name_length;
 
   if (def_symbol_in_progress != NULL)
     {
@@ -604,9 +603,7 @@ obj_coff_def (int what ATTRIBUTE_UNUSED)
   SKIP_WHITESPACES ();
 
   name_end = get_symbol_name (&symbol_name);
-  symbol_name_length = strlen (symbol_name);
-  symbol_name_copy = xmalloc (symbol_name_length + 1);
-  strcpy (symbol_name_copy, symbol_name);
+  symbol_name_copy = xstrdup (symbol_name);
 #ifdef tc_canonicalize_symbol_name
   symbol_name_copy = tc_canonicalize_symbol_name (symbol_name_copy);
 #endif
@@ -1083,11 +1080,7 @@ weak_is_altname (const char * name)
 static const char *
 weak_name2altname (const char * name)
 {
-  char *alt_name;
-
-  alt_name = xmalloc (sizeof (weak_altprefix) + strlen (name));
-  strcpy (alt_name, weak_altprefix);
-  return strcat (alt_name, name);
+  return concat (weak_altprefix, name, (char *) NULL);
 }
 
 /* Return the name of the weak symbol corresponding to an
@@ -1115,11 +1108,7 @@ weak_uniquify (const char * name)
 #endif
   gas_assert (weak_is_altname (name));
 
-  ret = xmalloc (strlen (name) + strlen (unique) + 2);
-  strcpy (ret, name);
-  strcat (ret, ".");
-  strcat (ret, unique);
-  return ret;
+  return concat (name, ".", unique, (char *) NULL);
 }
 
 void
@@ -1562,8 +1551,7 @@ obj_coff_section (int ignore ATTRIBUTE_UNUSED)
     }
 
   c = get_symbol_name (&section_name);
-  name = xmalloc (input_line_pointer - section_name + 1);
-  strcpy (name, section_name);
+  name = xstrdup (section_name);
   *input_line_pointer = c;
   SKIP_WHITESPACE_AFTER_NAME ();
 
@@ -1820,9 +1808,7 @@ obj_coff_init_stab_section (segT seg)
   /* Zero it out.  */
   memset (p, 0, 12);
   file = as_where ((unsigned int *) NULL);
-  stabstr_name = xmalloc (strlen (seg->name) + 4);
-  strcpy (stabstr_name, seg->name);
-  strcat (stabstr_name, "str");
+  stabstr_name = concat (seg->name, "str", (char *) NULL);
   stroff = get_stab_string_offset (file, stabstr_name);
   know (stroff == 1);
   md_number_to_chars (p, stroff, 4);
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index ea9f7ab..1404443 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -949,9 +949,7 @@ obj_elf_section_name (void)
 	  return NULL;
 	}
 
-      name = (char *) xmalloc (end - input_line_pointer + 1);
-      memcpy (name, input_line_pointer, end - input_line_pointer);
-      name[end - input_line_pointer] = '\0';
+      name = xstrndup (input_line_pointer, end - input_line_pointer);
 
       while (flag_sectname_subst)
         {
@@ -2060,9 +2058,7 @@ obj_elf_init_stab_section (segT seg)
   /* Zero it out.  */
   memset (p, 0, 12);
   file = as_where (NULL);
-  stabstr_name = (char *) xmalloc (strlen (segment_name (seg)) + 4);
-  strcpy (stabstr_name, segment_name (seg));
-  strcat (stabstr_name, "str");
+  stabstr_name = concat (segment_name (seg), "str", (char *) NULL);
   stroff = get_stab_string_offset (file, stabstr_name);
   know (stroff == 1 || (stroff == 0 && file[0] == '\0'));
   md_number_to_chars (p, stroff, 4);
diff --git a/gas/config/obj-evax.c b/gas/config/obj-evax.c
index e368592..aa925a9 100644
--- a/gas/config/obj-evax.c
+++ b/gas/config/obj-evax.c
@@ -273,9 +273,7 @@ evax_shorten_name (char *id)
     }
 
   /* We only need worry about krunching the base symbol.  */
-  base_id = xmalloc (suffix_dotdot - prefix_dotdot + 1);
-  strncpy (base_id, &id[prefix_dotdot], suffix_dotdot - prefix_dotdot);
-  base_id [suffix_dotdot - prefix_dotdot] = 0;
+  base_id = xstrndup (&id[prefix_dotdot], suffix_dotdot - prefix_dotdot);
 
   if (strlen (base_id) > MAX_LABEL_LENGTH)
     {
@@ -299,8 +297,7 @@ evax_shorten_name (char *id)
 	strcat (new_id, suffix);
 
       /* Save it on the heap and return.  */
-      return_id = xmalloc (strlen (new_id) + 1);
-      strcpy (return_id, new_id);
+      return_id = xstrdup (new_id);
 
       return return_id;
     }
diff --git a/gas/config/obj-macho.c b/gas/config/obj-macho.c
index f823b5c..ba079f3 100644
--- a/gas/config/obj-macho.c
+++ b/gas/config/obj-macho.c
@@ -206,16 +206,7 @@ obj_mach_o_make_or_get_sect (char * segname, char * sectname,
       /* There is no normal BFD section name for this section.  Create one.
          The name created doesn't really matter as it will never be written
          on disk.  */
-      size_t seglen = strlen (segname);
-      size_t sectlen = strlen (sectname);
-      char *n;
-
-      n = xmalloc (seglen + 1 + sectlen + 1);
-      memcpy (n, segname, seglen);
-      n[seglen] = '.';
-      memcpy (n + seglen + 1, sectname, sectlen);
-      n[seglen + 1 + sectlen] = 0;
-      name = n;
+      name = concat (segname, ".", sectname, (char *) NULL);
       if (specified_mask & SECT_TYPE_SPECIFIED)
 	sectype = usectype;
       else
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 8fb93ee..4b5099f 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -1241,9 +1241,7 @@ create_register_alias (char *newname, char *p)
   nlen = strlen (newname);
 #endif
 
-  nbuf = xmalloc (nlen + 1);
-  memcpy (nbuf, newname, nlen);
-  nbuf[nlen] = '\0';
+  nbuf = xstrndup (newname, nlen);
 
   /* Create aliases under the new name as stated; an all-lowercase
      version of the new name; and an all-uppercase version of the new
diff --git a/gas/config/tc-alpha.c b/gas/config/tc-alpha.c
index 89eaf88..970e06f 100644
--- a/gas/config/tc-alpha.c
+++ b/gas/config/tc-alpha.c
@@ -1417,9 +1417,7 @@ load_expression (int targreg,
 		    ptr1 = strstr (symname, "..") + 2;
 		    if (ptr1 > ptr2)
 		      ptr1 = symname;
-		    psymname = (char *) xmalloc (ptr2 - ptr1 + 1);
-		    memcpy (psymname, ptr1, ptr2 - ptr1);
-		    psymname [ptr2 - ptr1] = 0;
+		    psymname = xstrndup (ptr1, ptr2 - ptr1);
 
 		    gas_assert (insn.nfixups + 1 <= MAX_INSN_FIXUPS);
 		    insn.fixups[insn.nfixups].reloc = BFD_RELOC_ALPHA_LDA;
@@ -3959,9 +3957,7 @@ s_alpha_file (int ignore ATTRIBUTE_UNUSED)
       discard_rest_of_line ();
 
       len = input_line_pointer - start;
-      first_file_directive = (char *) xmalloc (len + 1);
-      memcpy (first_file_directive, start, len);
-      first_file_directive[len] = '\0';
+      first_file_directive = xstrndup (start, len);
 
       input_line_pointer = start;
     }
@@ -4214,9 +4210,7 @@ s_alpha_section_name (void)
 	  return NULL;
 	}
 
-      name = xmalloc (end - input_line_pointer + 1);
-      memcpy (name, input_line_pointer, end - input_line_pointer);
-      name[end - input_line_pointer] = '\0';
+      name = xstrndup (input_line_pointer, end - input_line_pointer);
       input_line_pointer = end;
     }
   SKIP_WHITESPACE ();
@@ -6283,10 +6277,7 @@ tc_gen_reloc (asection *sec ATTRIBUTE_UNUSED,
       if (pname_len > 4 && strcmp (pname + pname_len - 4, "..en") == 0)
 	{
 	  symbolS *sym;
-	  char *my_pname = (char *) xmalloc (pname_len - 4 + 1);
-
-	  memcpy (my_pname, pname, pname_len - 4);
-	  my_pname [pname_len - 4] = 0;
+	  char *my_pname = my_pname = xstrndup (pname, pname_len - 4);
 	  sym = symbol_find (my_pname);
 	  free (my_pname);
 	  if (sym == NULL)
diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 7486924..af02cfb 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -2253,9 +2253,7 @@ md_assemble (char *str)
 
   /* Split off the opcode.  */
   opnamelen = strspn (str, "abcdefghijklmnopqrstuvwxyz_0123468");
-  opname = xmalloc (opnamelen + 1);
-  memcpy (opname, str, opnamelen);
-  opname[opnamelen] = '\0';
+  opname = xstrndup (str, opnamelen);
 
   /* Signalize we are assmbling the instructions.  */
   assembling_insn = TRUE;
diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 4d16603..cc54a94 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -2254,9 +2254,7 @@ create_register_alias (char * newname, char *p)
   nlen = strlen (newname);
 #endif
 
-  nbuf = xmalloc (nlen + 1);
-  memcpy (nbuf, newname, nlen);
-  nbuf[nlen] = '\0';
+  nbuf = xstrndup (newname, nlen);
 
   /* Create aliases under the new name as stated; an all-lowercase
      version of the new name; and an all-uppercase version of the new
@@ -2419,9 +2417,7 @@ create_neon_reg_alias (char *newname, char *p)
   namelen = strlen (newname);
 #endif
 
-  namebuf = xmalloc (namelen + 1);
-  strncpy (namebuf, newname, namelen);
-  namebuf[namelen] = '\0';
+  namebuf = xstrndup (newname, namelen);
 
   insert_neon_reg_alias (namebuf, basereg->number, basetype,
 			 typeinfo.defined != 0 ? &typeinfo : NULL);
@@ -21866,10 +21862,7 @@ start_unwind_section (const segT text_seg, int idx)
   const char * prefix;
   const char * prefix_once;
   const char * group_name;
-  size_t prefix_len;
-  size_t text_len;
   char * sec_name;
-  size_t sec_name_len;
   int type;
   int flags;
   int linkonce;
@@ -21898,13 +21891,7 @@ start_unwind_section (const segT text_seg, int idx)
       text_name += strlen (".gnu.linkonce.t.");
     }
 
-  prefix_len = strlen (prefix);
-  text_len = strlen (text_name);
-  sec_name_len = prefix_len + text_len;
-  sec_name = (char *) xmalloc (sec_name_len + 1);
-  memcpy (sec_name, prefix, prefix_len);
-  memcpy (sec_name + prefix_len, text_name, text_len);
-  sec_name[prefix_len + text_len] = '\0';
+  sec_name = concat (prefix, text_name, (char *) NULL);
 
   flags = SHF_ALLOC;
   linkonce = 0;
diff --git a/gas/config/tc-hppa.c b/gas/config/tc-hppa.c
index 6bf1bba..1009173 100644
--- a/gas/config/tc-hppa.c
+++ b/gas/config/tc-hppa.c
@@ -5974,11 +5974,8 @@ pa_build_unwind_subspace (struct call_info *call_info)
   /* Replace the start symbol with a local symbol that will be reduced
      to a section offset.  This avoids problems with weak functions with
      multiple definitions, etc.  */
-  name = xmalloc (strlen ("L$\001start_")
-		  + strlen (S_GET_NAME (call_info->start_symbol))
-		  + 1);
-  strcpy (name, "L$\001start_");
-  strcat (name, S_GET_NAME (call_info->start_symbol));
+  name = concat ("L$\001start_", S_GET_NAME (call_info->start_symbol),
+		 (char *) NULL);
 
   /* If we have a .procend preceded by a .exit, then the symbol will have
      already been defined.  In that case, we don't want another unwind
@@ -6414,6 +6411,7 @@ hppa_elf_mark_end_of_function (void)
   /* ELF does not have EXIT relocations.  All we do is create a
      temporary symbol marking the end of the function.  */
   char *name;
+      symbolS *symbolP;
 
   if (last_call_info == NULL || last_call_info->start_symbol == NULL)
     {
@@ -6422,48 +6420,37 @@ hppa_elf_mark_end_of_function (void)
       return;
     }
 
-  name = xmalloc (strlen ("L$\001end_")
-		  + strlen (S_GET_NAME (last_call_info->start_symbol))
-		  + 1);
-  if (name)
-    {
-      symbolS *symbolP;
-
-      strcpy (name, "L$\001end_");
-      strcat (name, S_GET_NAME (last_call_info->start_symbol));
-
-      /* If we have a .exit followed by a .procend, then the
-	 symbol will have already been defined.  */
-      symbolP = symbol_find (name);
-      if (symbolP)
-	{
-	  /* The symbol has already been defined!  This can
-	     happen if we have a .exit followed by a .procend.
-
-	     This is *not* an error.  All we want to do is free
-	     the memory we just allocated for the name and continue.  */
-	  xfree (name);
-	}
-      else
-	{
-	  /* symbol value should be the offset of the
-	     last instruction of the function */
-	  symbolP = symbol_new (name, now_seg, (valueT) (frag_now_fix () - 4),
-				frag_now);
+  name = concat ("L$\001end_", S_GET_NAME (last_call_info->start_symbol),
+		 (char *) NULL);
 
-	  gas_assert (symbolP);
-	  S_CLEAR_EXTERNAL (symbolP);
-	  symbol_table_insert (symbolP);
-	}
+  /* If we have a .exit followed by a .procend, then the
+     symbol will have already been defined.  */
+  symbolP = symbol_find (name);
+  if (symbolP)
+    {
+      /* The symbol has already been defined!  This can
+	 happen if we have a .exit followed by a .procend.
 
-      if (symbolP)
-	last_call_info->end_symbol = symbolP;
-      else
-	as_bad (_("Symbol '%s' could not be created."), name);
+	 This is *not* an error.  All we want to do is free
+	 the memory we just allocated for the name and continue.  */
+      xfree (name);
+    }
+  else
+    {
+      /* symbol value should be the offset of the
+	 last instruction of the function */
+      symbolP = symbol_new (name, now_seg, (valueT) (frag_now_fix () - 4),
+			    frag_now);
 
+      gas_assert (symbolP);
+      S_CLEAR_EXTERNAL (symbolP);
+      symbol_table_insert (symbolP);
     }
+
+  if (symbolP)
+    last_call_info->end_symbol = symbolP;
   else
-    as_bad (_("No memory for symbol name."));
+    as_bad (_("Symbol '%s' could not be created."), name);
 }
 #endif
 
diff --git a/gas/config/tc-nios2.c b/gas/config/tc-nios2.c
index bf37ff7..1677d1d 100644
--- a/gas/config/tc-nios2.c
+++ b/gas/config/tc-nios2.c
@@ -3159,10 +3159,7 @@ nios2_modify_arg (char **parsed_args, const char *modifier,
 {
   char *tmp = parsed_args[ndx];
 
-  parsed_args[ndx]
-    = (char *) malloc (strlen (parsed_args[ndx]) + strlen (modifier) + 1);
-  strcpy (parsed_args[ndx], tmp);
-  strcat (parsed_args[ndx], modifier);
+  parsed_args[ndx] = concat (tmp, modifier, (char *) NULL);
 }
 
 /* Modify parsed_args[ndx] by negating that argument.  */
@@ -3172,13 +3169,7 @@ nios2_negate_arg (char **parsed_args, const char *modifier ATTRIBUTE_UNUSED,
 {
   char *tmp = parsed_args[ndx];
 
-  parsed_args[ndx]
-    = (char *) malloc (strlen ("~(") + strlen (parsed_args[ndx]) +
-		       strlen (")+1") + 1);
-
-  strcpy (parsed_args[ndx], "~(");
-  strcat (parsed_args[ndx], tmp);
-  strcat (parsed_args[ndx], ")+1");
+  parsed_args[ndx] = concat ("~(", tmp, ")+1", (char *) NULL);
 }
 
 /* The function nios2_swap_args swaps the pointers at indices index_1 and
diff --git a/gas/config/tc-rx.c b/gas/config/tc-rx.c
index e0af5b5..dced3ce 100644
--- a/gas/config/tc-rx.c
+++ b/gas/config/tc-rx.c
@@ -531,10 +531,7 @@ rx_section (int ignore)
 
       if (*p != '"' && *p != '#')
 	{
-	  char * name = (char *) xmalloc (len + 1);
-
-	  strncpy (name, input_line_pointer, len);
-	  name[len] = 0;
+	  char *name = xstrndup (input_line_pointer, len);
 
 	  input_line_pointer = p;
 	  parse_rx_section (name);
diff --git a/gas/config/tc-sh64.c b/gas/config/tc-sh64.c
index eb1c287..ec08173 100644
--- a/gas/config/tc-sh64.c
+++ b/gas/config/tc-sh64.c
@@ -3273,8 +3273,7 @@ sh64_consume_datalabel (const char *name, expressionS *exp,
 	    {
 	      symbolS *dl_symp;
 	      const char * sname = S_GET_NAME (symp);
-	      char *dl_name
-		= xmalloc (strlen (sname) + sizeof (DATALABEL_SUFFIX));
+	      char *dl_name = concat (sname, DATALABEL_SUFFIX, (char *) NULL);
 
 	      /* Now we copy the datalabel-qualified symbol into a symbol
 		 with the same name, but with " DL" appended.  We mark the
@@ -3282,8 +3281,6 @@ sh64_consume_datalabel (const char *name, expressionS *exp,
 		 the main symbol, so we don't have to inspect all symbol
 		 names.  Note that use of "datalabel" is not expected to
 		 be a common case.  */
-	      strcpy (dl_name, sname);
-	      strcat (dl_name, DATALABEL_SUFFIX);
 
 	      /* A FAKE_LABEL_NAME marks "$" or ".".  There can be any
 		 number of them and all have the same (faked) name; we
diff --git a/gas/config/tc-tic30.c b/gas/config/tc-tic30.c
index 725691c..c3e1c8f 100644
--- a/gas/config/tc-tic30.c
+++ b/gas/config/tc-tic30.c
@@ -380,11 +380,8 @@ tic30_find_parallel_insn (char *current_line, char *next_line)
 	}
       }
   }
-  parallel_insn = malloc (strlen (first_opcode) + strlen (first_operands)
-			  + strlen (second_opcode) + strlen (second_operands) + 8);
-  sprintf (parallel_insn, "q_%s_%s %s | %s",
-	   first_opcode, second_opcode,
-	   first_operands, second_operands);
+parallel_insn = concat ("q_", first_opcode, "_", second_opcode, " ",
+		       	first_operands, " | ", second_operands, (char *) NULL);
   debug ("parallel insn = %s\n", parallel_insn);
   return parallel_insn;
 }
diff --git a/gas/config/tc-tic54x.c b/gas/config/tc-tic54x.c
index 5dd772e..2671d56 100644
--- a/gas/config/tc-tic54x.c
+++ b/gas/config/tc-tic54x.c
@@ -1932,8 +1932,7 @@ tic54x_include (int ignored ATTRIBUTE_UNUSED)
      and a .newblock.
      The included file will be inserted before the newblock, so that the
      newblock is executed after the included file is processed.  */
-  input = xmalloc (sizeof (newblock) + strlen (filename) + 4);
-  sprintf (input, "\"%s\"\n%s", filename, newblock);
+  input = concat ("\"", filename, "\"\n", newblock, (char *) NULL);
   input_scrub_insert_line (input);
 
   tic54x_clear_local_labels (0);
@@ -2518,10 +2517,8 @@ tic54x_macro_info (const macro_entry *macro)
   /* Put the formal args into the substitution symbol table.  */
   for (entry = macro->formals; entry; entry = entry->next)
     {
-      char *name = strncpy (xmalloc (entry->name.len + 1),
-			    entry->name.ptr, entry->name.len);
-      char *value = strncpy (xmalloc (entry->actual.len + 1),
-			     entry->actual.ptr, entry->actual.len);
+      char *name = xstrndup (entry->name.ptr, entry->name.len);
+      char *value = xstrndup (entry->actual.ptr, entry->actual.len);
 
       name[entry->name.len] = '\0';
       value[entry->actual.len] = '\0';
@@ -4293,9 +4290,7 @@ subsym_get_arg (char *line, const char *terminators, char **str, int nosub)
       while (ISDIGIT (*ptr))
 	++ptr;
       endp = ptr;
-      *str = xmalloc (ptr - line + 1);
-      strncpy (*str, line, ptr - line);
-      (*str)[ptr - line] = 0;
+      *str = xstrndup (line, ptr - line);
     }
   else if (is_string)
     {
@@ -4327,9 +4322,7 @@ subsym_get_arg (char *line, const char *terminators, char **str, int nosub)
 	    ++term;
 	}
       endp = ptr;
-      *str = xmalloc (ptr - line + 1);
-      strncpy (*str, line, ptr - line);
-      (*str)[ptr - line] = 0;
+      *str = xstrndup (line, ptr - line);
       /* Do simple substitution, if available.  */
       if (!nosub && (value = subsym_lookup (*str, macro_level)) != NULL)
 	*str = value;
@@ -4452,8 +4445,7 @@ subsym_substitute (char *line, int forced)
 	      continue;
 	    }
 	  *ptr++ = '\0';
-	  tmp = xmalloc (strlen (head) + 2 + strlen (ptr) + 1);
-	  sprintf (tmp, "%s==%s", head, ptr);
+	  tmp = concat (head, "==", ptr, (char *) NULL);
 	  /* Continue examining after the '=='.  */
 	  ptr = tmp + strlen (head) + 2;
 	  free (replacement);
@@ -4751,9 +4743,7 @@ tic54x_start_line_hook (void)
   endp = input_line_pointer;
   while (!is_end_of_line[(int) *endp++])
     ;
-  line = xmalloc (endp - input_line_pointer + 1);
-  strncpy (line, input_line_pointer, endp - input_line_pointer + 1);
-  line[endp - input_line_pointer] = 0;
+  line = xstrndup (input_line_pointer, endp - input_line_pointer);
 
   /* Scan ahead for parallel insns.  */
   parallel_on_next_line_hint = next_line_shows_parallel (endp);
diff --git a/gas/config/tc-xtensa.c b/gas/config/tc-xtensa.c
index b7d1582..22181dc 100644
--- a/gas/config/tc-xtensa.c
+++ b/gas/config/tc-xtensa.c
@@ -1594,10 +1594,7 @@ xtensa_literal_prefix (void)
 		"abcdefghijklmnopqrstuvwxyz_/0123456789.$");
 
   /* Get a null-terminated copy of the name.  */
-  name = xmalloc (len + 1);
-  gas_assert (name);
-  strncpy (name, input_line_pointer, len);
-  name[len] = 0;
+  name = xstrndup (input_line_pointer, len);
 
   /* Skip the name in the input line.  */
   input_line_pointer += len;
@@ -2239,8 +2236,7 @@ xg_reverse_shift_count (char **cnt_argp)
   cnt_arg = *cnt_argp;
 
   /* replace the argument with "31-(argument)" */
-  new_arg = (char *) xmalloc (strlen (cnt_arg) + 6);
-  sprintf (new_arg, "31-(%s)", cnt_arg);
+  new_arg = concat ("31-(", cnt_argp, ")", (char *) NULL);
 
   free (cnt_arg);
   *cnt_argp = new_arg;
diff --git a/gas/config/xtensa-relax.c b/gas/config/xtensa-relax.c
index 3adbf2a..ac616a1 100644
--- a/gas/config/xtensa-relax.c
+++ b/gas/config/xtensa-relax.c
@@ -820,9 +820,7 @@ enter_opname_n (const char *name, int len)
 	return op->opname;
     }
   op = (opname_e *) xmalloc (sizeof (opname_e));
-  op->opname = (char *) xmalloc (len + 1);
-  strncpy (op->opname, name, len);
-  op->opname[len] = '\0';
+  op->opname = xstrndup (name, len);
   return op->opname;
 }
 
@@ -1127,9 +1125,7 @@ split_string (split_rec *rec,
       else
 	{
 	  len = p - q;
-	  rec->vec[i] = (char *) xmalloc (sizeof (char) * (len + 1));
-	  strncpy (rec->vec[i], q, len);
-	  rec->vec[i][len] = '\0';
+	  rec->vec[i] = xstrndup (q, len);
 	  p++;
 	}
 
diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
index 27b2646..3cee7d6 100644
--- a/gas/dwarf2dbg.c
+++ b/gas/dwarf2dbg.c
@@ -515,9 +515,7 @@ get_filenum (const char *filename, unsigned int num)
 	      dirs = XRESIZEVEC (char *, dirs, dirs_allocated);
 	    }
 
-	  dirs[dir] = (char *) xmalloc (dir_len + 1);
-	  memcpy (dirs[dir], filename, dir_len);
-	  dirs[dir][dir_len] = '\0';
+	  dirs[dir] = xstrndup (filename, dir_len);
 	  dirs_in_use = dir + 1;
 	}
     }
@@ -1297,11 +1295,7 @@ process_entries (segT seg, struct line_entry *e)
       sec_name = bfd_get_section_name (stdoutput, seg);
       if (strcmp (sec_name, ".text") != 0)
 	{
-	  unsigned int len;
-
-	  len = strlen (sec_name);
-	  name = xmalloc (len + 11 + 2);
-	  sprintf (name, ".debug_line%s", sec_name);
+	  name = concat (".debug_line", sec_name, (char *) NULL);
 	  subseg_set (subseg_get (name, FALSE), 0);
 	}
       else
diff --git a/gas/expr.c b/gas/expr.c
index afe46c4..b1cdb38 100644
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -1155,11 +1155,8 @@ operand (expressionS *expressionP, enum expr_mode mode)
 	      SKIP_WHITESPACE ();
 	      c = get_symbol_name (& name);
 
-	      buf = (char *) xmalloc (strlen (name) + 10);
-	      if (start)
-		sprintf (buf, ".startof.%s", name);
-	      else
-		sprintf (buf, ".sizeof.%s", name);
+	      buf = concat (start ? ".startof." : ".sizeof.", name,
+			    (char *) NULL);
 	      symbolP = symbol_make (buf);
 	      free (buf);
 
@@ -1301,11 +1298,8 @@ operand (expressionS *expressionP, enum expr_mode mode)
 
 	      c = get_symbol_name (& name);
 
-	      buf = (char *) xmalloc (strlen (name) + 10);
-	      if (start)
-		sprintf (buf, ".startof.%s", name);
-	      else
-		sprintf (buf, ".sizeof.%s", name);
+	      buf = concat (start ? ".startof." : ".sizeof.", name,
+			    (char *) NULL);
 	      symbolP = symbol_make (buf);
 	      free (buf);
 
diff --git a/gas/itbl-ops.c b/gas/itbl-ops.c
index 2c09f71..1a4e2bb 100644
--- a/gas/itbl-ops.c
+++ b/gas/itbl-ops.c
@@ -857,9 +857,7 @@ alloc_entry (e_processor processor, e_type type,
   if (e)
     {
       memset (e, 0, sizeof (struct itbl_entry));
-      e->name = (char *) malloc (sizeof (strlen (name)) + 1);
-      if (e->name)
-	strcpy (e->name, name);
+      e->name = xstrdup (name);
       e->processor = processor;
       e->type = type;
       e->value = value;
diff --git a/gas/listing.c b/gas/listing.c
index c90c575..07090d7 100644
--- a/gas/listing.c
+++ b/gas/listing.c
@@ -234,11 +234,8 @@ listing_message (const char *name, const char *message)
 {
   if (listing_tail != (list_info_type *) NULL)
     {
-      unsigned int l = strlen (name) + strlen (message) + 1;
-      char *n = (char *) xmalloc (l);
+      char *n = concat (name, message, (char *) NULL);
       struct list_message *lm = XNEW (struct list_message);
-      strcpy (n, name);
-      strcat (n, message);
       lm->message = n;
       lm->next = NULL;
 
@@ -1555,9 +1552,7 @@ listing_title (int depth)
 	  if (listing)
 	    {
 	      length = input_line_pointer - start;
-	      ttl = (char *) xmalloc (length + 1);
-	      memcpy (ttl, start, length);
-	      ttl[length] = 0;
+	      ttl = xstrndup (start, length);
 	      listing_tail->edict = depth ? EDICT_SBTTL : EDICT_TITLE;
 	      listing_tail->edict_arg = ttl;
 	    }
diff --git a/gas/macro.c b/gas/macro.c
index 615bfda..851454d 100644
--- a/gas/macro.c
+++ b/gas/macro.c
@@ -1250,9 +1250,7 @@ check_macro (const char *line, sb *expand,
   if (is_name_ender (*s))
     ++s;
 
-  copy = (char *) xmalloc (s - line + 1);
-  memcpy (copy, line, s - line);
-  copy[s - line] = '\0';
+  copy = xstrndup (line, s - line);
   for (cls = copy; *cls != '\0'; cls ++)
     *cls = TOLOWER (*cls);
 
diff --git a/gas/stabs.c b/gas/stabs.c
index 08b8439..10e271f 100644
--- a/gas/stabs.c
+++ b/gas/stabs.c
@@ -429,9 +429,7 @@ s_xstab (int what)
      the stab section name.  */
   if (saved_secname == 0 || strcmp (saved_secname, stab_secname))
     {
-      stabstr_secname = (char *) xmalloc (strlen (stab_secname) + 4);
-      strcpy (stabstr_secname, stab_secname);
-      strcat (stabstr_secname, "str");
+      stabstr_secname = concat (stab_secname, "str", (char *) NULL);
       if (saved_secname)
 	{
 	  free (saved_secname);
diff --git a/gas/symbols.c b/gas/symbols.c
index 4c3137a..0d5a854 100644
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -3220,25 +3220,16 @@ symbol_relc_make_expr (expressionS * exp)
 
   if (opstr == NULL)
     concat_string = NULL;
+  else if (arity == 0)
+    concat_string = xstrdup (opstr);
+  else if (arity == 1)
+    concat_string = concat (opstr, ":", operands[0], (char *) NULL);
+  else if (arity == 2)
+    concat_string = concat (opstr, ":", operands[0], ":", operands[1],
+			    (char *) NULL);
   else
-    {
-      /* Allocate new string; include inter-operand padding gaps etc.  */
-      concat_string = xmalloc (strlen (opstr)
-			       + 1
-			       + (arity >= 1 ? (strlen (operands[0]) + 1 ) : 0)
-			       + (arity >= 2 ? (strlen (operands[1]) + 1 ) : 0)
-			       + (arity >= 3 ? (strlen (operands[2]) + 0 ) : 0)
-			       + 1);
-      gas_assert (concat_string != NULL);
-
-      /* Format the thing.  */
-      sprintf (concat_string,
-	       (arity == 0 ? "%s" :
-		arity == 1 ? "%s:%s" :
-		arity == 2 ? "%s:%s:%s" :
-		/* arity == 3 */ "%s:%s:%s:%s"),
-	       opstr, operands[0], operands[1], operands[2]);
-    }
+    concat_string = concat (opstr, ":", operands[0], ":", operands[1], ":",
+			    operands[2], (char *) NULL);
 
   /* Free operand strings (not opstr).  */
   if (arity >= 1) xfree (operands[0]);
diff --git a/gas/write.c b/gas/write.c
index 15330cf..c965e68 100644
--- a/gas/write.c
+++ b/gas/write.c
@@ -1545,10 +1545,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED)
   gas_assert (x);
   if (!compression_header_size)
     {
-      compressed_name = (char *) xmalloc (strlen (section_name) + 2);
-      compressed_name[0] = '.';
-      compressed_name[1] = 'z';
-      strcpy (compressed_name + 2, section_name + 1);
+      compressed_name = concat (".z", section_name + 1, (char *) NULL);
       bfd_section_name (stdoutput, sec) = compressed_name;
     }
 }
-- 
2.1.4

Re: [PATCH] use xstrdup and concat more

[Nick Clifton]

Hi Trev,

> gas/ChangeLog:
> 2016-04-24  Trevor Saunders  <tbsaunde+binutils@tbsaunde.org>
> 
> 	* config/obj-coff.c (obj_coff_def): Simplify string copying.
> 	(weak_name2altname): Likewise.
> 	(weak_uniquify): Likewise.
> 	(obj_coff_section): Likewise.
> 	(obj_coff_init_stab_section): Likewise.
> 	* config/obj-elf.c (obj_elf_section_name): Likewise.
> 	(obj_elf_init_stab_section): Likewise.
> 	* config/obj-evax.c (evax_shorten_name): Likewise.
> 	* config/obj-macho.c (obj_mach_o_make_or_get_sect): Likewise.
> 	* config/tc-aarch64.c (create_register_alias): Likewise.
> 	* config/tc-alpha.c (load_expression): Likewise.
> 	(s_alpha_file): Likewise.
> 	(s_alpha_section_name): Likewise.
> 	(tc_gen_reloc): Likewise.
> 	* config/tc-arc.c (md_assemble): Likewise.
> 	* config/tc-arm.c (create_neon_reg_alias): Likewise.
> 	(start_unwind_section): Likewise.
> 	* config/tc-hppa.c (pa_build_unwind_subspace): Likewise.
> 	(hppa_elf_mark_end_of_function): Likewise.
> 	* config/tc-nios2.c (nios2_modify_arg): Likewise.
> 	(nios2_negate_arg): Likewise.
> 	* config/tc-rx.c (rx_section): Likewise.
> 	* config/tc-sh64.c (sh64_consume_datalabel): Likewise.
> 	* config/tc-tic30.c (tic30_find_parallel_insn): Likewise.
> 	* config/tc-tic54x.c (tic54x_include): Likewise.
> 	(tic54x_macro_info): Likewise.
> 	(subsym_get_arg): Likewise.
> 	(subsym_substitute): Likewise.
> 	(tic54x_start_line_hook): Likewise.
> 	* config/tc-xtensa.c (xtensa_literal_prefix): Likewise.
> 	(xg_reverse_shift_count): Likewise.
> 	* config/xtensa-relax.c (enter_opname_n): Likewise.
> 	(split_string): Likewise.
> 	* dwarf2dbg.c (get_filenum): Likewise.
> 	(process_entries): Likewise.
> 	* expr.c (operand): Likewise.
> 	* itbl-ops.c (alloc_entry): Likewise.
> 	* listing.c (listing_message): Likewise.
> 	(listing_title): Likewise.
> 	* macro.c (check_macro): Likewise.
> 	* stabs.c (s_xstab): Likewise.
> 	* symbols.c (symbol_relc_make_expr): Likewise.
> 	* write.c (compress_debug): Likewise.
 
Approved - please apply.

Cheers
  Nick

Re: [PATCH] use xstrdup and concat more

[Alan Modra]

On Sun, Apr 24, 2016 at 03:44:56AM -0400, tbsaunde+binutils@tbsaunde.org wrote:
> --- a/gas/config/obj-elf.c
> +++ b/gas/config/obj-elf.c
> @@ -949,9 +949,7 @@ obj_elf_section_name (void)
>  	  return NULL;
>  	}
>  
> -      name = (char *) xmalloc (end - input_line_pointer + 1);
> -      memcpy (name, input_line_pointer, end - input_line_pointer);
> -      name[end - input_line_pointer] = '\0';
> +      name = xstrndup (input_line_pointer, end - input_line_pointer);
>  
>        while (flag_sectname_subst)
>          {

Is this a good idea, here, and in other places where the original uses
memcpy and strlen was not called to find the string length?  I'm
thinking that xstrndup will be needlessly calling strlen.

Hmm, maybe use xmemdup?  Mind you, the xmemdup implementation isn't
ideal due to zeroing with xcalloc, but that could be fixed.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] use xstrdup and concat more

[Trevor Saunders]

On Mon, Apr 25, 2016 at 10:31:36PM +0930, Alan Modra wrote:
> On Sun, Apr 24, 2016 at 03:44:56AM -0400, tbsaunde+binutils@tbsaunde.org wrote:
> > --- a/gas/config/obj-elf.c
> > +++ b/gas/config/obj-elf.c
> > @@ -949,9 +949,7 @@ obj_elf_section_name (void)
> >  	  return NULL;
> >  	}
> >  
> > -      name = (char *) xmalloc (end - input_line_pointer + 1);
> > -      memcpy (name, input_line_pointer, end - input_line_pointer);
> > -      name[end - input_line_pointer] = '\0';
> > +      name = xstrndup (input_line_pointer, end - input_line_pointer);
> >  
> >        while (flag_sectname_subst)
> >          {
> 
> Is this a good idea, here, and in other places where the original uses
> memcpy and strlen was not called to find the string length?  I'm
> thinking that xstrndup will be needlessly calling strlen.

I guess that's true, I'm not sure if that really matters though?

> Hmm, maybe use xmemdup?  Mind you, the xmemdup implementation isn't
> ideal due to zeroing with xcalloc, but that could be fixed.

 We'd still have to be careful to null terminate ourselves, but I guess
 that would work.  Changing xmemdup scares me a little, but I can't
 think of a way it could be incorrect to use xmalloc there.

 Trev

> 
> -- 
> Alan Modra
> Australia Development Lab, IBM

Re: [PATCH] use xstrdup and concat more

[Alan Modra]

On Mon, Apr 25, 2016 at 09:55:50AM -0400, Trevor Saunders wrote:
> On Mon, Apr 25, 2016 at 10:31:36PM +0930, Alan Modra wrote:
> > On Sun, Apr 24, 2016 at 03:44:56AM -0400, tbsaunde+binutils@tbsaunde.org wrote:
> > > --- a/gas/config/obj-elf.c
> > > +++ b/gas/config/obj-elf.c
> > > @@ -949,9 +949,7 @@ obj_elf_section_name (void)
> > >  	  return NULL;
> > >  	}
> > >  
> > > -      name = (char *) xmalloc (end - input_line_pointer + 1);
> > > -      memcpy (name, input_line_pointer, end - input_line_pointer);
> > > -      name[end - input_line_pointer] = '\0';
> > > +      name = xstrndup (input_line_pointer, end - input_line_pointer);
> > >  
> > >        while (flag_sectname_subst)
> > >          {
> > 
> > Is this a good idea, here, and in other places where the original uses
> > memcpy and strlen was not called to find the string length?  I'm
> > thinking that xstrndup will be needlessly calling strlen.
> 
> I guess that's true, I'm not sure if that really matters though?

Quite possibly not.  I wasn't rejecting the patch and didn't see
anything in the patch that raised a red flag.  It was more a case
of asking you to think about possible performance effects when tidying
code.  Fewer lines of code is not always better.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] use xstrdup and concat more

[Trevor Saunders]

On Tue, Apr 26, 2016 at 12:26:29AM +0930, Alan Modra wrote:
> On Mon, Apr 25, 2016 at 09:55:50AM -0400, Trevor Saunders wrote:
> > On Mon, Apr 25, 2016 at 10:31:36PM +0930, Alan Modra wrote:
> > > On Sun, Apr 24, 2016 at 03:44:56AM -0400, tbsaunde+binutils@tbsaunde.org wrote:
> > > > --- a/gas/config/obj-elf.c
> > > > +++ b/gas/config/obj-elf.c
> > > > @@ -949,9 +949,7 @@ obj_elf_section_name (void)
> > > >  	  return NULL;
> > > >  	}
> > > >  
> > > > -      name = (char *) xmalloc (end - input_line_pointer + 1);
> > > > -      memcpy (name, input_line_pointer, end - input_line_pointer);
> > > > -      name[end - input_line_pointer] = '\0';
> > > > +      name = xstrndup (input_line_pointer, end - input_line_pointer);
> > > >  
> > > >        while (flag_sectname_subst)
> > > >          {
> > > 
> > > Is this a good idea, here, and in other places where the original uses
> > > memcpy and strlen was not called to find the string length?  I'm
> > > thinking that xstrndup will be needlessly calling strlen.
> > 
> > I guess that's true, I'm not sure if that really matters though?
> 
> Quite possibly not.  I wasn't rejecting the patch and didn't see
> anything in the patch that raised a red flag.  It was more a case
> of asking you to think about possible performance effects when tidying
> code.  Fewer lines of code is not always better.

Of course ;)  I think most of it is around section names which I imagine
isn't very hot.  What would you like to do here?

Trev

> 
> -- 
> Alan Modra
> Australia Development Lab, IBM

Re: [PATCH] use xstrdup and concat more

[Alan Modra]

On Tue, Apr 26, 2016 at 07:58:43PM -0400, Trevor Saunders wrote:
> On Tue, Apr 26, 2016 at 12:26:29AM +0930, Alan Modra wrote:
> > On Mon, Apr 25, 2016 at 09:55:50AM -0400, Trevor Saunders wrote:
> > > On Mon, Apr 25, 2016 at 10:31:36PM +0930, Alan Modra wrote:
> > > > On Sun, Apr 24, 2016 at 03:44:56AM -0400, tbsaunde+binutils@tbsaunde.org wrote:
> > > > > --- a/gas/config/obj-elf.c
> > > > > +++ b/gas/config/obj-elf.c
> > > > > @@ -949,9 +949,7 @@ obj_elf_section_name (void)
> > > > >  	  return NULL;
> > > > >  	}
> > > > >  
> > > > > -      name = (char *) xmalloc (end - input_line_pointer + 1);
> > > > > -      memcpy (name, input_line_pointer, end - input_line_pointer);
> > > > > -      name[end - input_line_pointer] = '\0';
> > > > > +      name = xstrndup (input_line_pointer, end - input_line_pointer);
> > > > >  
> > > > >        while (flag_sectname_subst)
> > > > >          {
> > > > 
> > > > Is this a good idea, here, and in other places where the original uses
> > > > memcpy and strlen was not called to find the string length?  I'm
> > > > thinking that xstrndup will be needlessly calling strlen.
> > > 
> > > I guess that's true, I'm not sure if that really matters though?
> > 
> > Quite possibly not.  I wasn't rejecting the patch and didn't see
> > anything in the patch that raised a red flag.  It was more a case
> > of asking you to think about possible performance effects when tidying
> > code.  Fewer lines of code is not always better.
> 
> Of course ;)  I think most of it is around section names which I imagine
> isn't very hot.  What would you like to do here?

How about using a new function, called xmemdup0, say?  That way you
can happily tidy the source without needing to worry about possible
performance effects (and no one needs to worry when reviewing).

I'm about to commit the following.  inline is handled by ansidecl.h,
and the obstack.h macros no longer used.

	* as.h (inline): Don't define.
	(__PTR_TO_INT, __INT_TO_PTR): Don't define.
	(xmemdup0): New inline function.

diff --git a/gas/as.h b/gas/as.h
index 9ff8bb8..f3e1cf0 100644
--- a/gas/as.h
+++ b/gas/as.h
@@ -98,13 +98,6 @@
 /* Define the standard progress macros.  */
 #include "progress.h"
 
-/* This doesn't get taken care of anywhere.  */
-#ifndef __MWERKS__  /* Metrowerks C chokes on the "defined (inline)"  */
-#if !defined (__GNUC__) && !defined (inline)
-#define inline
-#endif
-#endif /* !__MWERKS__ */
-
 /* Other stuff from config.h.  */
 #ifdef NEED_DECLARATION_ENVIRON
 extern char **environ;
@@ -144,14 +137,6 @@ extern int vsnprintf(char *, size_t, const char *, va_list);
 #define bcopy(src,dest,size)	memcpy (dest, src, size)
 #endif
 
-/* Make Saber happier on obstack.h.  */
-#ifdef SABER
-#undef  __PTR_TO_INT
-#define __PTR_TO_INT(P) ((int) (P))
-#undef  __INT_TO_PTR
-#define __INT_TO_PTR(P) ((char *) (P))
-#endif
-
 #ifndef __LINE__
 #define __LINE__ "unknown"
 #endif /* __LINE__ */
@@ -522,6 +507,14 @@ segT   subseg_get (const char *, int);
 const char *remap_debug_filename (const char *);
 void add_debug_prefix_map (const char *);
 
+static inline void *
+xmemdup0 (const void *in, size_t len)
+{
+  char *out = (char *) xmalloc (len + 1);
+  out[len] = 0;
+  return memcpy (out, in, len);
+}
+
 struct expressionS;
 struct fix;
 typedef struct symbol symbolS;

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] use xstrdup and concat more

[Trevor Saunders]

On Wed, Apr 27, 2016 at 03:49:25PM +0930, Alan Modra wrote:
> On Tue, Apr 26, 2016 at 07:58:43PM -0400, Trevor Saunders wrote:
> > On Tue, Apr 26, 2016 at 12:26:29AM +0930, Alan Modra wrote:
> > > On Mon, Apr 25, 2016 at 09:55:50AM -0400, Trevor Saunders wrote:
> > > > On Mon, Apr 25, 2016 at 10:31:36PM +0930, Alan Modra wrote:
> > > > > On Sun, Apr 24, 2016 at 03:44:56AM -0400, tbsaunde+binutils@tbsaunde.org wrote:
> > > > > > --- a/gas/config/obj-elf.c
> > > > > > +++ b/gas/config/obj-elf.c
> > > > > > @@ -949,9 +949,7 @@ obj_elf_section_name (void)
> > > > > >  	  return NULL;
> > > > > >  	}
> > > > > >  
> > > > > > -      name = (char *) xmalloc (end - input_line_pointer + 1);
> > > > > > -      memcpy (name, input_line_pointer, end - input_line_pointer);
> > > > > > -      name[end - input_line_pointer] = '\0';
> > > > > > +      name = xstrndup (input_line_pointer, end - input_line_pointer);
> > > > > >  
> > > > > >        while (flag_sectname_subst)
> > > > > >          {
> > > > > 
> > > > > Is this a good idea, here, and in other places where the original uses
> > > > > memcpy and strlen was not called to find the string length?  I'm
> > > > > thinking that xstrndup will be needlessly calling strlen.
> > > > 
> > > > I guess that's true, I'm not sure if that really matters though?
> > > 
> > > Quite possibly not.  I wasn't rejecting the patch and didn't see
> > > anything in the patch that raised a red flag.  It was more a case
> > > of asking you to think about possible performance effects when tidying
> > > code.  Fewer lines of code is not always better.
> > 
> > Of course ;)  I think most of it is around section names which I imagine
> > isn't very hot.  What would you like to do here?
> 
> How about using a new function, called xmemdup0, say?  That way you
> can happily tidy the source without needing to worry about possible
> performance effects (and no one needs to worry when reviewing).

Sounds good with my one comment below, thanks!

> I'm about to commit the following.  inline is handled by ansidecl.h,
> and the obstack.h macros no longer used.
> 
> 	* as.h (inline): Don't define.
> 	(__PTR_TO_INT, __INT_TO_PTR): Don't define.
> 	(xmemdup0): New inline function.
> 
> diff --git a/gas/as.h b/gas/as.h
> index 9ff8bb8..f3e1cf0 100644
> --- a/gas/as.h
> +++ b/gas/as.h
> @@ -98,13 +98,6 @@
>  /* Define the standard progress macros.  */
>  #include "progress.h"
>  
> -/* This doesn't get taken care of anywhere.  */
> -#ifndef __MWERKS__  /* Metrowerks C chokes on the "defined (inline)"  */
> -#if !defined (__GNUC__) && !defined (inline)
> -#define inline
> -#endif
> -#endif /* !__MWERKS__ */
> -
>  /* Other stuff from config.h.  */
>  #ifdef NEED_DECLARATION_ENVIRON
>  extern char **environ;
> @@ -144,14 +137,6 @@ extern int vsnprintf(char *, size_t, const char *, va_list);
>  #define bcopy(src,dest,size)	memcpy (dest, src, size)
>  #endif
>  
> -/* Make Saber happier on obstack.h.  */
> -#ifdef SABER
> -#undef  __PTR_TO_INT
> -#define __PTR_TO_INT(P) ((int) (P))
> -#undef  __INT_TO_PTR
> -#define __INT_TO_PTR(P) ((char *) (P))
> -#endif
> -
>  #ifndef __LINE__
>  #define __LINE__ "unknown"
>  #endif /* __LINE__ */
> @@ -522,6 +507,14 @@ segT   subseg_get (const char *, int);
>  const char *remap_debug_filename (const char *);
>  void add_debug_prefix_map (const char *);
>  
> +static inline void *
> +xmemdup0 (const void *in, size_t len)
> +{
> +  char *out = (char *) xmalloc (len + 1);
> +  out[len] = 0;
> +  return memcpy (out, in, len);

Given we always use this with strings, and null terminating other things
with 1 zero byte doesn't seem to make a lot of sense should this return
char *?  I can make that change  with my patch if you agree.

Trev

Re: [PATCH] use xstrdup and concat more

[Alan Modra]

On Wed, Apr 27, 2016 at 06:20:53AM -0400, Trevor Saunders wrote:
> > +static inline void *
> > +xmemdup0 (const void *in, size_t len)
> > +{
> > +  char *out = (char *) xmalloc (len + 1);
> > +  out[len] = 0;
> > +  return memcpy (out, in, len);
> 
> Given we always use this with strings, and null terminating other things
> with 1 zero byte doesn't seem to make a lot of sense should this return
> char *?  I can make that change  with my patch if you agree.

I chose void * to be like xmemdup, but as you say, the function will
probably only be used to copy strings.  Feel free to change the param
and result to char * (and cast memcpy result).

-- 
Alan Modra
Australia Development Lab, IBM