[haiku-commits] Re: r40720 - in haiku/trunk: headers/os/support src/kits/support
- From: Oliver Tappe <zooey@xxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sun, 27 Feb 2011 13:13:06 +0100
On 2011-02-27 at 10:25:26 [+0100], jonas@xxxxxxxxxxx wrote:
> Author: kirilla
> Date: 2011-02-27 10:25:26 +0100 (Sun, 27 Feb 2011)
> New Revision: 40720
> Changeset: http://dev.haiku-os.org/changeset/40720
>
> Modified:
> haiku/trunk/headers/os/support/String.h
> haiku/trunk/src/kits/support/String.cpp
> Log:
> Cleanup. Efficiency makerover.
[ ... ]
>
> Modified: haiku/trunk/src/kits/support/String.cpp
> ===================================================================
> --- haiku/trunk/src/kits/support/String.cpp 2011-02-27 08:34:08 UTC (rev
> 40719)
> +++ haiku/trunk/src/kits/support/String.cpp 2011-02-27 09:25:26 UTC (rev
> 40720)
> @@ -425,20 +425,30 @@
>
>
> BString&
> -BString::SetToArguments(const char *format, ...)
> +BString::SetToArguments(const char* format, ...)
> {
> + int32 bufferSize = 128;
> + char buffer[bufferSize];
> +
> va_list arg;
> va_start(arg, format);
> - int32 bytes = vsnprintf(LockBuffer(0), 0, format, arg);
> + int32 bytes = vsnprintf(buffer, bufferSize, format, arg);
> va_end(arg);
> - UnlockBuffer(0);
I know we're on the verge of nitpicking here, but don't just believe what
Axel says ;-)
Using an on-stack buffer isn't so cheap after all, since that buffer has to
be copied into the string when it happens to have been large enough. Instead,
I would just use the string's current buffer:
int32 bytes = vsnprintf(LockBuffer(0), Length() + 1, format, arg);
That has the additional advantage that the caller of this method can
influence the initial size of the buffer beforehand.
> +
> + if (bytes < 0) {
> + return *this;
> + }
no braces needed
> - va_list arg2;
> - va_start(arg2, format);
> - bytes = vsnprintf(LockBuffer(bytes), bytes + 1, format, arg2);
> - va_end(arg2);
> - UnlockBuffer(bytes);
> -
> + if (bytes < bufferSize) {
> + SetTo(buffer);
> + } else {
Again, no braces required, but I can see that it looks nicer because of the
else part having them. Should we perhaps add that to our styleguide?
cheers,
Oliver
Other related posts:
