Opened 16 years ago

Closed 16 years ago

Last modified 21 months ago

#8337 closed patch

ScummVM 0.6.0 fixes when not using GCC

Reported by: SF/mrhandler Owned by: fingolfin
Priority: normal Component: Port: IRIX
Keywords: Cc:
Game:

Description

During the build of ScummVM 0.6.0 with MIPSPro C/C++ compilers on SGI IRIX, I've found some errors and some incompabilities that I've fixed (see the attached patch file).

- backends/fs/fs.h does not define the access mode for the parent class Common::List<FilesystemNode *> (fixed to private, as implied by the compiler)

- scumm/insane/insane.h & scumm/insane/insane.cpp have a wrong dimension for the matrix _enemyState, which yelds to memory overwritings

- some missing scope qualifiers in scumm/dialogs.cpp have been reinstated

- compilers different from GCC can be really picky, but in generally pretend that a function which should return a value does it in any possible branch of its body.

Hope it helps, Andrea

Ticket imported from: #924685. Ticket imported from: patches/442.

Attachments (1)

scummvm-patches (7.7 KB ) - added by SF/mrhandler 16 years ago.
ScummVM 0.6.0 fixes (all platforms)

Download all attachments as: .zip

Change History (7)

by SF/mrhandler, 16 years ago

Attachment: scummvm-patches added

ScummVM 0.6.0 fixes (all platforms)

comment:1 by fingolfin, 16 years ago

* Please make patches against HEAD CVS, not 0.6.0

* It's not a bug to omit "private" in a class declaration or in class inheritance; it's in fact a core C++ feature that the default is "private".. Do you mean to say the MIPSPro C/C++ doesn't support this? Wow. File a bug report with the MIPSPro vendor already :-)

* The scope qualifiers in scumm/dialogs.cpp aren't really missing, they are implied by context (as per the C++ spec). I don't mind adding them, though, if that helps broken compilers :-)

* As for the functions where you added "return 0;" statements -> error() is marked as a NO-RETURN function on GCC, so it knows now to trigger a return. I'd prefer if such a defintion could be added for MIPSPro, too, but if it doesn't support that, we could add those return's... Although, strictly spoken, those are merely warnings :-)

* The insane array size indeed is wrong, thanks for catching this nasty bug!

* The SDAT loading code looks plain wrong, adding a memset there fixes the symptom, not the cause - I'll go and talk to the author of that piece of code to find out what the proper fix is.

comment:2 by fingolfin, 16 years ago

Component: --Unset--

comment:3 by SF/mrhandler, 16 years ago

OK, next time it will be against CVS. For the rest of comments, just a clarification: it's not that MIPSPro is broken (although it may be as well, sometime), MIPSPro C/C++ compilers are only much more picky than GCC and, on SGI machines at least, they produce code that is way over faster than GCC.

Most if not all the fixes are in reality warnings, including the private thing: the compiler just hinted that it chose the default access mode, private, which could have been an overlook by the user. The same is valid for the missing return statements.

And, BTW, it's not me that catched the nasty bug in the array, it was the compiler. Now talk about "broken" compilers ;)

comment:4 by fingolfin, 16 years ago

Well... as for the "broken" part: you called this tracker item "fixes when not using GCC"... to me a fix solves an error, hence I was assuming MIPSPro was giving you errors - which indeed would have implied that it was broken. If those were just pedantic warnings, that's fine. Although they are indeed quite pedantic.

I added the "private" and the GUI:: specifiers anyway, just to make you happy, although I see 0 purpose in it. :-)

Furthermpre, I am very grateful for the fix in scumm/insane/insane.h, that might fix some odd errors in the INSANE code.

As for the added return statements, I am not willing to add those. If there is a way in MIPSPro to specify that a function is a no-return function (like you can do with GCC, MSVC, and various other compilers), please tell us and we'll be happy to add support for it; otherwise, I am afraid you'll have to live with those warnings (maybe you can just turn them off).

comment:5 by fingolfin, 16 years ago

Owner: set to fingolfin
Status: newclosed

comment:6 by digitall, 21 months ago

Component: Port: IRIX
Note: See TracTickets for help on using tickets.