This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] use xstrdup and concat more
- From: Alan Modra <amodra at gmail dot com>
- To: Trevor Saunders <tbsaunde at tbsaunde dot org>
- Cc: tbsaunde+binutils at tbsaunde dot org, binutils at sourceware dot org
- Date: Wed, 27 Apr 2016 15:49:25 +0930
- Subject: Re: [PATCH] use xstrdup and concat more
- Authentication-results: sourceware.org; auth=none
- References: <1461483896-3190-1-git-send-email-tbsaunde+binutils at tbsaunde dot org> <20160425130136 dot GH27353 at bubble dot grove dot modra dot org> <20160425135550 dot GE25520 at tsaunders-iceball dot corp dot tor1 dot mozilla dot com> <20160425145629 dot GJ27353 at bubble dot grove dot modra dot org> <20160426235843 dot GC9731 at ball>
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