Opened 17 years ago

Closed 17 years ago

Last modified 12 months ago

#8053 closed patch

Inappopriate error() exit approach

Reported by: SF/jamieson630 Owned by: SF/ender
Priority: normal Component: --Other--
Keywords: Cc:


The error() function used far and wide across ScummVM
attempts to exit the program using exit(1). This type of
bailout fails to properly shut down supporting systems
and often results in processor faults (seg fault, page
fault, etc).

This patch replaces exit(1) with a call to the system
shutdown function OSystem_SDL_Common::quit().

Note that there are 33 other active occurrences of exit(1)
within ScummVM, and they may also consitute
inappropriate bailouts that cause processor faults. It
may be advisable to review these to see if a cleaner exit
approach could be applied.

Ticket imported from: #610251. Ticket imported from: patches/158.

Attachments (1)

scummvm.diff (517 bytes ) - added by SF/jamieson630 17 years ago.
Patch against 1.16 revision

Download all attachments as: .zip

Change History (5)

by SF/jamieson630, 17 years ago

Attachment: scummvm.diff added

Patch against 1.16 revision

comment:1 by fingolfin, 17 years ago

Funny, I was talking just today about this on #scummvm :-)

Another reason why we would want orderly exit is so that we can
properly clean up resources, i.e. delete the Engine/Config objects which
can in turn delete all their resources -> makes it much easier to detect
memory leaks, but on the negative side slows down program exits.

One possible approach would be to use exceptions for this, but I am not
sure if all our targets support those (I know that some embedded C++
compilers don't). But considering that we use templates, I guess it would
be OK.

Why exceptions and not just calling some common quit() method or
setting a global flag? Because we may have to exit at any point, so a
flag won't work always correctly, and a global quit() function might have
a hard time to clean up properly. Of course it still would be possible, but
excpetions will also clean up the stack... let's ponder this a bit.

comment:2 by SF/ender, 17 years ago

Owner: set to SF/ender
Status: newclosed

comment:3 by SF/ender, 17 years ago

For the moment I've just went and replaced a few of the exit()s with a call
to OSystem->quit().

We can come up with a better cleanup method later :)

comment:4 by digitall, 12 months ago

Component: --Other--
Note: See TracTickets for help on using tickets.