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

[Max Horn]

Hi there,

and yet another proposal by me, this time relevant for everybody. As always, feedback is highly welcome. 

If I hear no relevant objections, this will be gradually implemented as-is over time.

Proposed change:
---------------
We should replace all uses of printf and fprintf (at least in non-backend code) by uses of our own text console functions, as defined in common/textconsole.h
Specifically, all code currently using printf should be changed to use either a GUI dialog, error(), warning(), debug(), or *maybe* a new, yet-to-be-introduced function. I'll refer to it as "notify()" for the rest of this text, but the particular name is still up for debate.

The new notify() function can be viewed as a companion to error(), warning() and debug(). Whether we actually need it or not still has to be determined.

In addition, we should write a guidelines which explains when to use which: I.e. when to use warning(), when debug (and debugN, debugC, ...), when notify(), and when to show a GUI dialog instead.

Moreover, we should have a well-defined OSystem API which the code in common/textconsole.cpp and common/debug.cpp uses to perform actual text console output.

Rational:
--------
On many ports, there is no console, and porters have to either manually implement a printf substitute. By avoiding (f)printf, and instead relying on a well-defined OSystem API, we make it much simpler for porters to provide alternatives to console output, such as sending any console output to a file, or a serial device. The OSystem API would also allow us to get rid of most (if not all) system specific hacks in common/textconsole.cpp.
As an extra bonus, it becomes easy to send console output to multiple destinations simultaneously, e.g. to a log file and stdout.

Caveats:
-------
None.

Implementation:
--------------
We need to go over every use of printf and fprintf, at least in non-backend code, and change it to a warning, debug, error or notify call, or even remove it completely. Once that is done, printf and fprintf can be added to the list of forbidden symbols in common/forbidden.h.

During this, we can also determine whether notify will be necessary. If it is, then an initial implementation could rely on vprintf, or vsnprintf + fputs .

In the next stage, we would add new OSystem API for text console output, and would change common/textconsole.cpp and common/debug.cpp to use those. On a quick glance, it would seem that something like the following new OSystem methods would be needed:

1)  OSystem::logToConsole(ConsoleMsgType type, const char *msg);
Here ConsoleMsgType would be an enum specifying the type of output (e.g. ERROR, WARNING, DEBUG, PLAIN/NOTIFICATION). Backends could then handle different "types" of output differently (e.g. sending some to stdout and some to stderr; or logging them in different files; or prepending them with extra information; etc.).

2)  OSystem::fatalError()
This would by called as last thing by error(), and by default would invoke OSystem::quit() followed by exit(1). Look at the end of our current error() function in common/textconsole.cpp to get an idea of what other things backends might do there.
2b) Alternatively, this function could be part of OSystem::logToConsole, and triggered by message type FATAL_ERROR.

3) OSystem::flushLog()
This would correspond to fflush. We currently use this in debug().
3b) Alternatively, we could require that logToConsole performs no buffering resp. always does a flush.


Note that it'll be relatively easy to switch between the various design alternatives here, as the new API would only be invoked in two source files. So we can simply try out one and move to another if we change our mind later on.


Cheers,
Max