This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Allocate sufficient space for string buffer


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.

> -      max <<= 1;
> -      if (max == 0)
> +      max += dsize;

It's a bad idea to increase large buffers by a relatively small
amount.  You end up with lots of realloc calls.  That's why typical
buffer reallocation code multiplies by some factor, sometimes adding a
constant as well.  The multiply handles large buffers efficiently, the
addition makes small buffers increase at a faster rate.  I used
multiply by 2 simply because that was what sb.c used before my change.
Another thing: Ideally the initial size takes into account the
underlying malloc implementation.  For example the underlying
malloc might actually allocate a minimum of 64 bytes but use 8 bytes
of this itself for management, so the initial size ought to be 56.

If that were the case we might start with

static size_t dsize = 64 - sizeof (size_t) - 1;

and use

max = (max << 2) + sizeof (size_t) + 1;

to make the underlying malloc block always a power of two.  I haven't
checked exactly what glibc does..

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]