A lot of platforms have snprintf code which works even if the supplied buffer is NULL. eg snprintf(NULL, 0, format, ...) will give the size of the formatted string. The patch adds a few checks for str == NULL to prevent segfaults.
Created attachment 1311 [details] [review] allow snprintf to work on NULL buffer
Yay, go for it. I have the same change in one of my local trees. It quite useful, we should implement a xallocPrintf(const char *fmt, ...) which allocs the required space and snprintf()'s into it.
glibc has an asprintf(char **str, const char *format, ...) for this I'm about to use this implementation for XWin #ifndef HAS_VASPRINTF extern int vasprintf(char **strp, const char *format, va_list va) { int ret; int size; va_list va2; va_copy(va2, va); size = vsnprintf(NULL, 0, format, va2); va_end(va2); *strp = malloc(size + 1); if (*strp == NULL) return -1; ret = vsnprintf(*strp, size + 1, format, va); (*strp)[size] = 0; return ret; } #endif #ifndef HAS_ASPRINTF extern int asprintf(char **strp, const char *format, ...) { int ret; va_list va; va_start(va, format); ret = vasprintf(strp, format, va); va_end(va); return ret; } #endif
Yeah, that looks good. My only concern is that you're using malloc(), shouldn't that be xalloc()? Of course, in that case it wouldn't be asprintf() (which is documented as using malloc()), which is why I originally suggested xallocPrintf() ... for symmetry, I think a "no fail" xnfallocPrintf() would be cool, too.
Created attachment 1370 [details] [review] add Xprintf and XNFprintf to xserver Xprintf and Xvprintf allocate the buffer via Xalloc XNFprintf and XNFvprintf allocate it via XNFalloc should the macros x*printf be added? And how a simple #define xprintf Xprintf or more complicated like #define xvprintf(fmt, ...) Xvprintf(fmt, ???) is this portable? I think macros with variable arguments is a gnu extension
/cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.563; previous revision: 1.562 /cvs/xorg/xc/programs/Xserver/include/os.h,v <-- os.h new revision: 1.5; previous revision: 1.4 /cvs/xorg/xc/programs/Xserver/os/xprintf.c,v <-- xprintf.c initial revision: 1.1 /cvs/xorg/xc/programs/Xserver/os/Imakefile,v <-- Imakefile new revision: 1.7; previous revision: 1.6 /cvs/xorg/xc/lib/misc/snprintf.c,v <-- snprintf.c new revision: 1.3; previous revision: 1.2 marked as fixed...
Report from Felix Kühling: This commit broke the build for me, even with your later fix of the link order. The problem is that os/xprintf.c is built with -ansi -pedantic. In this case va_copy is not defined as a macro by stdarg.h (using gcc-3.3.4 and gcc-3.3.5) and is missing when Xorg is finally linked. va_copy not being defined is not a new problem, I found it in an older build log I had hanging around. The error was never found before because no other object linked into Xorg used Xprintf before this commit. So xprintf.o (from libos.a) was never linked. I'm not sure what the proper fix would be. Either change the compiler flags or avoid using va_copy in xprintf.c. -- according to bug #347 va_copy is defined as __va_copy
Created attachment 1827 [details] [review] check for va_copy not being defined and use __va_copy if available extends attachment #1370 [details] [review]
Comment on attachment 1827 [details] [review] check for va_copy not being defined and use __va_copy if available commited to head /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.751; previous revision: 1.750 /cvs/xorg/xc/programs/Xserver/os/xprintf.c,v <-- programs/Xserver/os/xprintf.c new revision: 1.3; previous revision: 1.2
marking as fixed
XFree has added a similar function Xasprintf. maybe renaming Xprintf to Xasprintf would be a good idea to have similar functions names and make porting of code easier
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.