This is the mail archive of the
mailing list for the binutils project.
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 != '\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;
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..
Australia Development Lab, IBM