[MPlayer-dev-eng] [PATCH] Add mp_strings.c with mp_asprintf function.

[Reimar Döffinger]

On Sat, Feb 05, 2011 at 10:20:37PM +0100, Clément Bœsch wrote:
> On Tue, Feb 01, 2011 at 11:19:02PM +0100, Clément Bœsch wrote:
> > On Thu, Jan 27, 2011 at 09:34:04PM +0100, Reimar Döffinger wrote:
> > > On Thu, Jan 27, 2011 at 09:11:25PM +0100, Clément Bœsch wrote:
> > > > > Sorry for the late reply. What I would expect might happen is that in
> > > > > C99 mode asprintf is not available (and we certainly don't want to
> > > > > have yet another thing that requires _GNU_SOURCE everywhere), so
> > > > > configure detection will fail. However linking will fail as well,
> > > > > since then asprintf is defined both in the libc and in MPlayer.
> > > > 
> > > > I'm not sure I get the difference between the compilation test made at
> > > > ./configure and the way mplayer is built; the asprintf test does not check
> > > > if we have _GNU_SOURCE defined, it try to compile a source code using it.
> > > > Or did I missed something again?
> > > 
> > > I realize it all doesn't make much sense, and I should just say:
> > > to get asprintf we need to define _GNU_SOURCE. But we absolutely
> > > should avoid this in any way possible. So even when we use asprintf
> > > we should do so via a wrapper, so that we have to define _GNU_SOURCE
> > > on in the one file where the wrapper is implemented.
> > > Just in case you are curious, my original thoughts were:
> > > Well, without _GNU_SOURCE it is not in the header. Ideally the test should
> > > fail (I just realize this probably is not the case) then since without a
> > > declaration it cannot be used safely, particularly on 64 bit systems.
> > > Thus we use our own implementation instead. That would be a bit silly
> > > in itself, since we would be using it always and never a system one.
> > > Either way, linking fails because that has the same name as a symbol in the libc.
> > > So we'd be stuck with defining _GNU_SOURCE everywhere where we used
> > > asprintf
> > 
> > Ok then, so let's go back to the first implementation? Patch re-attached…
> > 
> > [...]
> > 
> > From 64a5f85883b1cc513a969b735f9e3c6f03723659 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> > Date: Wed, 12 Jan 2011 23:37:11 +0100
> > Subject: [PATCH] Add mp_strings.c with mp_asprintf function.
> > 
> 
> Since no one can make a decision about this, I'll apply this last one in 3
> days. I don't mind changing all over again the code, but just tell me what
> you think.

It's fine by me in principle, except that I am still somewhat suspicious of
the va_* stuff (that used to be quite tricky/buggy at some points in time)
but if you think it really is useful I won't stop you.