Comment 14 for bug 1173090

Revision history for this message
Nelson Elhage (nelhage) wrote :

> However, I think that in the case of char[] variables, you can use &x
> for x, so this is unnecessary to remove the & operator (this remark
> also applies to oamread and xaread).

I think that is true (have not tested). I certainly find the version without & clearer, but it may be worth having a more minimal patch.

> Is a global necessary? If you put this line in copy_spc_data this
> should work, unless you have to put it in a different file to "trick"
> the static analysis.

Without testing, I suspect that GCC will "see through" the separate variable, and the FORTIFY_SOURCE checks will still kick in. Declaring it as a char[] and defining it in the .asm file (like I did for the others) might be cleaner, though.

> Anyway, something seems off with the size of this variable. Cppcheck
> detects an error with it:

Yeah, there are those problems and tons of others that GCC finds -- search https://launchpadlibrarian.net/142716062/buildlog_ubuntu-raring-i386.zsnes_1.510%2Bbz2-5ubuntu2fortify2_UPLOADING.txt.gz for "will always overflow destination buffer". I only fixed the ones that were necessary to make loading a save file work in my testing; Someone should probably dig into the rest, but I didn't have the time to understand the source code well enough to e.g. change the size in the declaration of SPCRAM and be confident that was safe.