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
next prev parent 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).