[Scummvm-devel] PROPOSAL: Removing (f)printf usage

[Max Horn]

Hi Julien, (CC: back to scummvm-devel as this concerns all).

Am 01.11.2010 um 15:38 schrieb Julien:

> On that subject, would it be possible to get rid of the hardcoded buffer
> size in ConsoleDialog::vprintf?

That would be desirable, but difficult, due to the fact that there is no va_copy available in general.

I'd also love to have a Common::String::vformat (i.e. a renamed version of String::vprintf from your patch). Everything else using a fixed size buffer plus vs(n)printf could then be rewritten to use that, once and for all... But portability is the primary issue here...

So, let's see how we can tackle this. Let's add va_copy support. We could start with something like the code here: <http://www.koders.com/c/fid574D1BFC65D3211F4C18F5CAE018760BAF5B432A.aspx?s=%22Jos%22>.

This suggests the following: In configure, check if va_copy is present. If so, define HAVE_VA_COPY.

Then, in scummsys.h (or maybe in a new common/vargs.h header ?), for targets that need a custom va_copy, add the relevant #defines. Finally, if !HAVE_VA_COPY and not defined(va_copy), then
#define va_copy(a,b) (a) = (b)

Note: On my system (Mac OS X with GCC), va_copy is defined as follows (in particular, is not *defined* when compiling ScummVM):

#if !defined(__STRICT_ANSI__) || __STDC_VERSION__ + 0 >= 199900L
#define va_copy(d,s)	__builtin_va_copy(d,s)
#endif
#define __va_copy(d,s)	__builtin_va_copy(d,s)




As a variation, we could name "our" va_copy something like scummvm_va_copy, to avoid any name clashes.



*Then* we can add Common::String::vformat etc.


> 
> There is a patch adding support for vprintf to Common::String
> (http://bitbucket.org/Littleboy/scummvm-jt/src/c0a14d21c1ef/COMMON%20-%20Add
> %20vprintf%20support%20to%20String%20class.patch) and  changes ConsoleDialog
> to use the new function
> (http://bitbucket.org/Littleboy/scummvm-jt/src/tip/GUI%20-%20Use%20vprintf%2
> 0in%20Console%20class.patch) but it needs C99 for va_copy (those two were
> briefly committed to trunk when merging the lastexpress engine and then
> backed out).

Cheers,
Max