While posix does require that vsnprintf return the number of characters that would have been put into the beffer less the trailing NULL byte, should the buffer have been big enough, not all systems do this. At least HP-UX, IRIX and Tru64 Unix have non-conforming vsnprintf, the HP-UX implementation returns -1 if the buffer is too small, the Tru64 and IRIX implementations return the number of chars actually copied into the buffer less the trailing NULL. This causes dbus to seg fault with 'dbus-daemin --introspect' on those platforms. Dbus can either use the gnulib implementation of vsnprintf, come up with its own implementation, or patch _dbus_printf_string_upper_bound.
*** 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
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.