Bruno Haible <[EMAIL PROTECTED]> writes:
> This is not needed. vasnprintf already does this check.
But my next bug report was going to be for vasnprintf, since
vasnprintf should not do the INT_MAX check. vasnprintf's API does not
suffer from the INT_MAX limit, and there's no need to inflict this
arbitrary limit on vasnprintf, asprintf, etc. Only functions like
snprintf, where the (poorly-designed, but cast-in-stone) API requires
that the result fit in int, should check for INT_MAX.
The first step to get this fixed is to change snprintf so that it does
the INT_MAX check. (This step is done in gnulib.) Assuming you
agree, the next step is to simplify/speedup vasnprintf by removing the
check there.
> gcc converts two comparisons like this to a single conditional jump only
> when both bounds are constant. Not when, like here, one of them is a
> variable.
Thanks, I stand corrected. (GCC should do better, but that's a bigger task....)
> Still one bug remaining: Your new code will crash when passed an str = NULL
> argument.
No, because if STR is null, then SIZE must be zero. C99 and POSIX
require this. So if there is a crash it's a user error.
> Furthermore I don't like that your memcpy copies more than needed.
> The vasnprintf code needs memory in chunks, is not looking for it byte
> for byte. Therefore it can happen that output != str and still
> 0 <= len < size - and then you are copying around more memory than needed.
Thanks, I didn't know that.
I installed the following patch to fix that, since it's the part we
all agree on so far. I'll await your opinion on the INT_MAX issue
before tackling it.
There's one other optimization that I thought of while reviewing this
change: don't take the address of LEN, so that the compiler can put
LEN into a register. This shrinks the size of the generated machine
code by 10% on my Debian x86 host with GCC 4.1.1 -O2. The following
patch incorporates that optimization too.
2006-08-11 Paul Eggert <[EMAIL PROTECTED]>
* lib/snprintf.c (snprintf): memcpy LEN bytes, not SIZE - 1, when
LEN is smaller than SIZE. Suggested by Bruno Haible.
Also, help the compiler to keep LEN in a register.
--- lib/snprintf.c 10 Aug 2006 19:32:38 -0000 1.8
+++ lib/snprintf.c 11 Aug 2006 17:47:58 -0000 1.10
@@ -45,11 +45,12 @@ snprintf (char *str, size_t size, const
{
char *output;
size_t len;
+ size_t lenbuf = size;
va_list args;
va_start (args, format);
- len = size;
- output = vasnprintf (str, &len, format, args);
+ output = vasnprintf (str, &lenbuf, format, args);
+ len = lenbuf;
va_end (args);
if (!output)
@@ -59,8 +60,9 @@ snprintf (char *str, size_t size, const
{
if (size)
{
- memcpy (str, output, size - 1);
- str[size - 1] = '\0';
+ size_t pruned_len = (len < size ? len : size - 1);
+ memcpy (str, output, pruned_len);
+ str[pruned_len] = '\0';
}
free (output);
