Re: [PATCH] use xstrdup and concat more

[Alan Modra <amodra at gmail dot com>]

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