public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: Trevor Saunders <tbsaunde@tbsaunde.org>
Cc: tbsaunde+binutils@tbsaunde.org, binutils@sourceware.org
Subject: Re: [PATCH] use xstrdup and concat more
Date: Wed, 27 Apr 2016 06:19:00 -0000	[thread overview]
Message-ID: <20160427061925.GB20333@bubble.grove.modra.org> (raw)
In-Reply-To: <20160426235843.GC9731@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

  reply	other threads:[~2016-04-27  6:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-24  7:38 tbsaunde+binutils
2016-04-25  8:40 ` Nick Clifton
2016-04-25 13:01 ` Alan Modra
2016-04-25 13:56   ` Trevor Saunders
2016-04-25 14:56     ` Alan Modra
2016-04-26 23:52       ` Trevor Saunders
2016-04-27  6:19         ` Alan Modra [this message]
2016-04-27 10:21           ` Trevor Saunders
2016-04-27 11:59             ` Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160427061925.GB20333@bubble.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=tbsaunde+binutils@tbsaunde.org \
    --cc=tbsaunde@tbsaunde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).