[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/11] Add append method to qstring and empty co
From: |
Jamie Lokier |
Subject: |
Re: [Qemu-devel] [PATCH 01/11] Add append method to qstring and empty constructor |
Date: |
Fri, 23 Oct 2009 20:33:43 +0100 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
Luiz Capitulino wrote:
> > qstring = qemu_malloc(sizeof(*qstring));
> > - qstring->string = qemu_strdup(str);
> > +
> > + qstring->length = strlen(str);
> > + qstring->capacity = qstring->length;
> > +
> > + qstring->string = qemu_malloc(qstring->capacity + 1);
> > + memcpy(qstring->string, str, qstring->length);
> > + qstring->string[qstring->length] = 0;
>
> Couldn't this be:
>
> qstring->string = qemu_strdup(str);
> qstring->length = qstring->capacity = strlen(str);
Probably to have one call to strlen() instead of two (one inside
qemu_strdup()).
> > +void qstring_append(QString *qstring, const char *str)
> > +{
> > + size_t len = strlen(str);
> > +
> > + if (qstring->capacity < (qstring->length + len)) {
> > + qstring->capacity += len;
> > + qstring->capacity *= 2; /* use exponential growth */
> > +
> > + qstring->string = qemu_realloc(qstring->string, qstring->capacity
> > + 1);
> > + }
>
> Why do we need to double it? Wouldn't be enough to only keep track
> of the current string length and add 'len' to it? We could drop
> 'capacity' then.
You need exponential growth if large stringes are to be grown in O(n)
time where n is the number of characters, appended in small pieces.
Think about the time spent copying bytes every time qemu_realloc() is called.
If you just add 'len' each time, think about appending 1 byte 10^4
times. It will copy approximately 10^8/2 bytes, which is a lot just to
make a string 10^4 bytes long.
But += len; *= 2 is not necessary. *= 2 is enough, provided the
result is large enough.
> > + memcpy(qstring->string + qstring->length, str, len);
> > + qstring->length += len;
> > + qstring->string[qstring->length] = 0;
>
> I would use strcat().
Again, that's an extra call to strlen(), traversing the string twice instead of
once.
Doesn't make much different for small strings, only large ones.
-- Jamie
- [Qemu-devel] [PATCH 0/11] json parser (v2), Anthony Liguori, 2009/10/17
- [Qemu-devel] [PATCH 01/11] Add append method to qstring and empty constructor, Anthony Liguori, 2009/10/17
- [Qemu-devel] [PATCH 02/11] Add support for qfloat, Anthony Liguori, 2009/10/17
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Luiz Capitulino, 2009/10/18
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Anthony Liguori, 2009/10/19
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Amit Shah, 2009/10/22
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Anthony Liguori, 2009/10/22
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Amit Shah, 2009/10/22
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Jamie Lokier, 2009/10/23
- Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat, Daniel P. Berrange, 2009/10/23
[Qemu-devel] [PATCH 03/11] Add a test case for qfloat, Anthony Liguori, 2009/10/17