#8846 closed patch
Memory leak fixes
| Reported by: | SF/gamblore | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | --Other-- |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Memory leak fixes in
ScummEngine_v5 and ScummEngine_v7 /engine/scumm/scumm.cpp adlib Sound sound/softsynth/adlib.cpp Evaluator gui/eval.cpp Singletons common/singleton.h Graphics scaler graphics/scaler.{cpp,h} SDL Backend backend/platform/sdl/sdl.cpp SDL Graphics backend/platform/sdl/graphics.cpp scummvm_main base/main.cpp
Approximately 1MB in memory leaks plugged.
I am unsure as to if I am destroying the graphics scaler buffers too soon.
Ticket imported from: #1925352. Ticket imported from: patches/951.
Attachments (1)
Change History (7)
by , 16 years ago
| Attachment: | memorypatches added |
|---|
comment:1 by , 16 years ago
Thanks, this is very much appreciated. However, instead/before just checking this in, I would like to get some things clarified:
* I am confused about the change to gui/eval.cpp. As it is, a HashMap *should* automatically free its content. Also, the ~Eval destructor should automatically call the destructors of all members. So, in theory, there should be no reason to call _strings.clear() in the ~Eval destructor. Your patch indicates that this is not true. If that is indeed the case, this hints at a deeper underlying problem, which should be identified and fixed properly once and for all. Instead of calling clear() manually everywhere.
* It is safe to call delete and free() on NULL pointers, so no need to check vars for being non-zero before deleting/freeing them
* You added a Singleton::destroy method, yet for the Config Manager (which is a singleton), you don't use that but rather call its destructor manually. Is that on purpose?
comment:2 by , 16 years ago
Alright heres what I was thinking... or not thinking in that manner.
The gui/eval.cpp .clears() are not needed... not sure what i was thinking. I must have got confused between reset() and the deconstructor somehow.
Yes don't really know why I did it really.
Yes this was a mistake. ConfMan.destroy() works.
comment:3 by , 16 years ago
No worries :). And ~Eval calls several clear() methods already, which are probably just as pointless, so no harm :) I just prefer to double check the real causes.
comment:4 by , 16 years ago
| Owner: | set to |
|---|---|
| Status: | new → closed |
comment:5 by , 16 years ago
Commited, with some further tweaks (Singleton::destroy sets the singleton pointer to 0)
comment:6 by , 5 years ago
| Component: | → --Other-- |
|---|
Patches for all files listed