This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: Allocate sufficient space for string buffer
On Fri, Jun 08, 2012 at 03:51:19AM -0700, H.J. Lu wrote:
> On Thu, Jun 7, 2012 at 9:16 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Thu, Jun 07, 2012 at 08:08:13PM -0700, H.J. Lu wrote:
> >> - ?sb_new (&from_sb);
> >> + ?/* Allocate sufficient space: from->len + optional newline + '\0'. ?*/
> >> + ?newline = from->len >= 1 && from->ptr[0] != '\n';
> >> + ?sb_build (&from_sb, from->len + newline + 1);
> >
> > No, please look at sb_build! ?It already adds one for the terminator.
>
> The extra 1 byte is for
>
> /* Make sure the parser looks at defined contents when it scans for
> e.g. end-of-line at the end of a macro. */
> sb_add_char (&from_sb, 0);
>
> since we have
>
> sb_add_char (&from_sb, '\n');
> sb_scrub_and_add_sb (&from_sb, from);
> sb_add_char (&from_sb, 0);
Sorry. I should have looked at the code rather than just your patch.
Hmm, that last sb_add_char ought to be sb_terminate, and sb_terminate
needs fixing. Then you will be able to do without the +1. OK with
that change. I'm applying the following.
* sb.c: Include limits.h.
(dsize): Delete.
(MALLOC_OVERHEAD, INIT_ALLOC): Define.
(sb_new): Use INIT_ALLOC.
(sb_check): Modify allocation strategy using MALLOC_OVERHEAD.
(sb_terminate): Don't use sb_add_char.
Index: gas/sb.c
===================================================================
RCS file: /cvs/src/src/gas/sb.c,v
retrieving revision 1.18
diff -u -p -r1.18 sb.c
--- gas/sb.c 7 Jun 2012 12:47:23 -0000 1.18
+++ gas/sb.c 9 Jun 2012 07:53:00 -0000
@@ -25,6 +25,13 @@
#include "as.h"
#include "sb.h"
+#ifdef HAVE_LIMITS_H
+#include <limits.h>
+#endif
+#ifndef CHAR_BIT
+#define CHAR_BIT 8
+#endif
+
/* These routines are about manipulating strings.
They are managed in things called `sb's which is an abbreviation
@@ -39,7 +46,13 @@
use foo->ptr[*];
sb_kill (&foo); */
-static size_t dsize = 32;
+/* Buffers start at INIT_ALLOC size, and roughly double each time we
+ go over the current allocation. MALLOC_OVERHEAD is a guess at the
+ system malloc overhead. We aim to not waste any memory in the
+ underlying page/chunk allocated by the system malloc. */
+#define MALLOC_OVERHEAD (2 * sizeof (size_t))
+#define INIT_ALLOC (64 - MALLOC_OVERHEAD - 1)
+
static void sb_check (sb *, size_t);
/* Initializes an sb. */
@@ -55,7 +68,7 @@ sb_build (sb *ptr, size_t size)
void
sb_new (sb *ptr)
{
- sb_build (ptr, dsize);
+ sb_build (ptr, INIT_ALLOC);
}
/* Deallocate the sb at ptr. */
@@ -114,16 +127,23 @@ sb_scrub_and_add_sb (sb *ptr, sb *s)
static void
sb_check (sb *ptr, size_t len)
{
- size_t max = ptr->max;
+ size_t want = ptr->len + len;
- while (ptr->len + len >= max)
+ if (want > ptr->max)
{
- max <<= 1;
- if (max == 0)
+ size_t max;
+
+ want += MALLOC_OVERHEAD + 1;
+ if ((ssize_t) want < 0)
as_fatal ("string buffer overflow");
- }
- if (max != ptr->max)
- {
+#if GCC_VERSION >= 3004
+ max = (size_t) 1 << (CHAR_BIT * sizeof (want) - __builtin_clzl (want));
+#else
+ max = 128;
+ while (want > max)
+ max <<= 1;
+#endif
+ max -= MALLOC_OVERHEAD + 1;
ptr->max = max;
ptr->ptr = xrealloc (ptr->ptr, max + 1);
}
@@ -167,13 +187,12 @@ sb_add_buffer (sb *ptr, const char *s, s
ptr->len += len;
}
-/* Like sb_name, but don't include the null byte in the string. */
+/* Write terminating NUL and return string. */
char *
sb_terminate (sb *in)
{
- sb_add_char (in, 0);
- --in->len;
+ in->ptr[in->len] = 0;
return in->ptr;
}
--
Alan Modra
Australia Development Lab, IBM