Bug 11668

*** Bug 16662 has been marked as a duplicate of this bug. ***

Does vsnprintf still do this in current versions of these OSs?

(In reply to comment #0)
> Dbus can either use the gnulib implementation of vsnprintf

It's LGPL, whereas D-Bus is under a bizarre dual-license which is unlikely to be changed (because one copyright holder is whoever received the assets of Codefactory AB when it folded), so no.

Something more permissively-licensed like Trio <http://daniel.haxx.se/projects/trio/> would be a possibility, though.

> come up with its own implementation

Life's too short, tbh.

> or patch _dbus_printf_string_upper_bound

It turns out the Windows implementation already has roughly the behaviour we want. Performance-wise, it's terrible (one malloc/free pair for each printf that doesn't fit in a buffer on the stack), but that's what you get for running on a pre-C99 libc.

Created attachment 48283 [details] [review]
[PATCH 1/2] Cope with platforms whose vsnprintf violates both POSIX  and C99

Created attachment 48284 [details] [review]
[PATCH 2/2] In Windows _dbus_printf_string_upper_bound, don't crash on malloc failure

(Cc Ralf for review of a Windows bugfix.)

(In reply to comment #5)
> Created an attachment (id=48284) [details]
> [PATCH 2/2] In Windows _dbus_printf_string_upper_bound, don't crash on malloc
> failure

This requires at least this part of patch 1:

--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -1255,6 +1255,9 @@ _dbus_string_append_printf_valist  (DBusString        *str
   /* Measure the message length without terminating nul */
   len = _dbus_printf_string_upper_bound (format, args);
 
+  if (len < 0)
+    return FALSE;
+
   if (!_dbus_string_lengthen (str, len))
     {
       /* don't leak the copy */

(In reply to comment #7)
> (In reply to comment #5)
> > Created an attachment (id=48284) [details] [details]
> > [PATCH 2/2] In Windows _dbus_printf_string_upper_bound, don't crash on malloc
> > failure
> 
> This requires at least this part of patch 1:
> 
> --- a/dbus/dbus-string.c
> +++ b/dbus/dbus-string.c
> @@ -1255,6 +1255,9 @@ _dbus_string_append_printf_valist  (DBusString       
> *str
>    /* Measure the message length without terminating nul */
>    len = _dbus_printf_string_upper_bound (format, args);
> 
> +  if (len < 0)
> +    return FALSE;
> +
>    if (!_dbus_string_lengthen (str, len))
>      {
>        /* don't leak the copy */

commited and pushed to git master

(In reply to comment #7)
> (In reply to comment #5)
> > Created an attachment (id=48284) [details] [details]
> > [PATCH 2/2] In Windows _dbus_printf_string_upper_bound, don't crash on malloc
> > failure
> 
> This requires at least this part of patch 1:

looks good - this patch and part 1 of the first patch committed

Created attachment 49614 [details] [review]
replacement patch, now that Ralf has applied 2/2 and half of 1/2

This is the bit that still needs review. I believe it should work for all platforms mentioned in the autoconf documentation, except those where the length is ignored completely (64-bit Solaris 7, apparently). In particular:

* Tru64/IRIX return the truncated length on truncation
* HP-UX returns negative on truncation

Anyone who still cares about Tru64, IRIX or HP-UX: please test this.

Anyone who cares about D-Bus being portable to ye olde proprietary Unix: please review this.

Review of attachment 49614 [details] [review]:

The great thing about standard APIs is that portability becomes so much easier. >_<

Assuming your breakdown of the different bugs in weird Unices is accurate, then I think this patch should work. The cost to Linux of this patch is the (unlikely) bufsize == len path making one redundant vsprintf call, and a couple of extra branches. Probably dwarfed by the cost of the original vsprintf call, I guess.

Fixed in git for 1.4.16 and 1.5.8, thanks