[Scummvm-devel] Re: [Scummvm-cvs-logs] SF.net SVN: scummvm: [20970] scummvm/trunk/engines/kyra/sound_adlib.cpp

[Torbjörn Andersson]

Marcus Comstedt wrote:

> Also note that it is _illegal_ to set the value of an object
> (including objects of type va_list...) more than once without a
> sequence point in between.  See §6.5:2 in the C standard.
> 
> Other operators that introduce a sequence points are ||, && and ?:.
> Function calls also introduce a sequence point, but only _after_ the
> call address and all the arguments have been evaluated.
> 
> I don't have a C++ standard handy, but I assume it follows the same
> rules as C here.

The problem, of course, is that no matter how many times I read the
relevant part of the C FAQ, I've never quite managed to get my head
around the exact implications. So I remain ignorant and paranoid. :-)

To pick a recent example, in ScummVM 0.7.1 we had this function:

void ScummEngine_v5::o5_jumpRelative() {
    _scriptPointer += (int16)fetchScriptWord();
}

This looks harmless, until you realize that very act of calling
fetchScriptWord() will modify _scriptPointer. Does this violate the
sequence point rule? I don't know. But I do know that it was one of the
things that kept ScummVM from being successfully compiled with GCC 4. I
was lucky enough that the first game I used to track down the bug
crashed immediately after executing this opcode, and it still took me
quite a while to realize what was happening.

If anyone knows for sure, feel free to update the big scary comment I
added to the function after changing it. :-)

Torbjörn