#4011 closed defect (fixed)
SAGA: Compiler warnings; possibly an engine bug
| Reported by: | eriktorbjorn | Owned by: | bluegr |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SAGA |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Current ScummVM SVN
While compiling ScummVM with the -O2 option, I encountered the following warning:
./engines/saga/animation.h: In member function ‘int Saga::Anim::playCutaway(int, bool)’: ./engines/saga/animation.h:173: warning: array subscript is above array bounds ./engines/saga/animation.h:173: warning: array subscript is above array bounds
Part of the problem is the validateAnimationsId() function in animation.h, which does
if (animId >= MAX_ANIMATIONS) { ... animId is a cutaway animation id } if (_animations[animId] == NULL) { error( ... ); }
Changing that to "} else if (_animations[animId] == NULL) {" brings up the next set of warnings:
./engines/saga/animation.h: In member function ‘int Saga::Anim::playCutaway(int, bool)’: ./engines/saga/animation.h:188: warning: array subscript is above array bounds ./engines/saga/animation.h:188: warning: array subscript is above array bounds
This time, it's the getAnimation() function that's the culprit. It says:
if (animId > MAX_ANIMATIONS) return _cutawayAnimations[animId - MAX_ANIMATIONS]; return _animations[animId];
Shouldn't that be "if (animId >= MAX_ANIMATIONS)" instead?
Changing that still leaves me with a warning:
./engines/saga/animation.h: In member function ‘int Saga::Anim::playCutaway(int, bool)’: ./engines/saga/animation.h:172: warning: array subscript is above array bounds
And this time I'm at a complete loss. Because now the validateAnimationId() function reads:
if (animId >= MAX_ANIMATIONS) { ... } else if (_animations[animId] == NULL) { ... }
It's the "else if" it complains about, but I can't see why. It's the call to setFrameTime() in playCutaway() that seems to trigger it.
Ticket imported from: #2284298. Ticket imported from: bugs/4011.
Change History (6)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
I made some of the corrections mentioned, could you please check if you're still getting warnings?
comment:3 by , 16 years ago
I still get the last warning I described:
./engines/saga/animation.h: In member function ‘int Saga::Anim::playCutaway(int, bool)’: ./engines/saga/animation.h:175: warning: array subscript is above array bounds
Which goes away if I remove the call to setFrameTime(). (Not a solution, of course, but I wanted to find out which call triggered it.) But it makes absolutely no sense to me. Maybe it's a false positive.
comment:5 by , 16 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:6 by , 6 years ago
| Component: | → Engine: SAGA |
|---|
..... void validateAnimationId(uint16 animId) { if (animId >= MAX_ANIMATIONS) { if (animId >= MAX_ANIMATIONS + ARRAYSIZE(_cutawayAnimations)) error("validateAnimationId: animId out of range"); if (_cutawayAnimations[animId - MAX_ANIMATIONS] == NULL) { error("validateAnimationId: animId=%i unassigned", animId); } } else if (_animations[animId] == NULL) { // check only if animId < MAX_ANIMATIONS error("validateAnimationId: animId=%i unassigned.", animId); } } ..... AnimationData* getAnimation(uint16 animId) { validateAnimationId(animId); if (animId >= MAX_ANIMATIONS) // '>=' definitely better than '>'. otherwise we can't access _cutawayAnimations[0] element return _cutawayAnimations[animId - MAX_ANIMATIONS]; return _animations[animId]; } .....
i think these changes are right. i was unable to reproduce any warning on my vs c++ /w4 compiler with above changes.