[PATCH 1/6 v2] Add X*asprintf() routines to mirror common asprintf() routines

Alan Coopersmith alan.coopersmith at oracle.com
Fri Dec 3 09:57:42 PST 2010


Keith Packard wrote:
> On Thu, 02 Dec 2010 19:49:21 -0800, Alan Coopersmith <alan.coopersmith at oracle.com> wrote:
> 
>> Would that be the best answer then?   Should the asprintf() replacement
>> just become _X_HIDDEN and link os/Xprintf.c into any module that calls
>> it?   Right now, it looks like libvbe.so is the only caller outside the
>> main server binary, and it calls XNFasprintf, which would be exported
>> ABI either way.
> 
> It would be lovely if we had a way of sharing only with 'internal'
> modules in the X server, but we don't. I also don't want to link os/Xprintf.c
> into multiple modules, that seems ugly.
> 
> So, it seems like we have the option of exporting asprintf in our
> effective ABI, or changing the name. Of the two, I'd be OK with
> exporting asprintf in our ABI, but I don't feel strongly about it. If
> someone has a cogent argument against providing asprintf in the ABI we
> provide to video or input drivers, I'd love to hear it.

I took a run at modifying the patches to do this in a new branch.

The biggest difficulty is dealing with the asprintf() prototype in the
header file.   I can wrap in in #ifndef HAVE_ASPRINTF but then either have
to guess which autoconf header to include for that (dix-config.h in most of
the server, xorg-server.h in drivers) or require all callers to include it
in an appropriate header beforehand.   If I don't wrap it, then we may clash
with system headers - for instance, I included the restrict keyword for the
format string argument since that's what Solaris <stdio.h> has, but not all
the man pages I see online for other OS'es mention that.

What if we took a middle path?

 - Have the function name exported by the X server be Xasprintf()

 - Have the Xprintf.h header export the API as:

	#include <stdio.h>

	extern _X_EXPORT int Xasprintf (char **ret,
                               const char * _X_RESTRICT_KYWD fmt,
                               ...) _X_ATTRIBUTE_PRINTF(2,3);

	#ifndef HAVE_ASPRINTF
	# define asprintf Xasprintf
	#endif

And then leave the patches to use it as calling just asprintf(), hiding
the complications in the header, and giving one place to update when we
decide it's time to just require it in the platform libc.

People who call asprintf() without having the right autoconf header first
may get a extra level of function call wrapping, but that's not a painful
problem.   It would mean that programmers would have to be more careful
about headers, since as long as you have <stdio.h> included on a system
with it, you won't get warned that you haven't included the header to
provide it to older systems, but as the xorg/os.h header is already
widely included, that may not happen too often.

-- 
	-Alan Coopersmith-        alan.coopersmith at oracle.com
	 Oracle Solaris Platform Engineering: X Window System



More information about the xorg-devel mailing list