Opened 16 years ago

Closed 16 years ago

Last modified 13 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, 13 months ago

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