[Scummvm-devel] incorrect use of stream functions

[Norbert Lange]

I played around with modification of endian.h and stream.h which yield a  
noteable savings in codesize (~230KB on AMD64, likely a fraction of that  
on other architectures), but I also had to realize that the stream.h class  
comments the functions with eg.:

/**
  * Read an unsigned byte from the stream and return it.
  * Performs no error checking. The return value is undefined
  * if a read error occurred (for which client code can check by
  * calling err() and eos() ).
  */
byte Stream::readByte()

Well, fine so far.. but there is code that explicitely tests the return  
value for 0 to detect end of stream and naturally breaks with unexpected  
behaviour if the Stream::readByte method returns something else. the  
xmplparser class does this everywhere.

So there are 3 ways to deal with that:
*) update the comments and make sure streams behave the defined way.
*) fix code depending on unspecified behaviour by make it using  
Stream::read(&val, 1)
*) the code that depends on Stream::readXX returning 0 for eos is surely  
reading character-streams, so keep the Stream::readXX returnin undefined  
values on eos, and define a function analog to C`s getchar() which returns  
-1 in case of error. And make the illbehaving code use that instead.

The solutions are sorted by the amount of work necessary, and I would  
prefer the last one myself. I can do a patch with the current behaviour of  
returning 0 in case of error (atleast for 8 bit read functions, the 16 and  
32 bit read variants shouldnt cause trouble) so the patch itself shouldnt  
affect the decision.

Cheers Norbert