Bug 1865

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