Source Code Project Mantis - FSSCP
View Issue Details
0003137FSSCPgameplaypublic2015-01-25 19:452015-06-29 17:02
Reporterchief1983 
Assigned ToEchelon9 
PrioritynormalSeverityminorReproducibilityN/A
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.7.1 
Target Version3.7.4Fixed in Version3.7.3 
Summary0003137: CID 1266279: Unchecked return value (CHECKED_RETURN)
Description*** CID 1266279: Unchecked return value (CHECKED_RETURN)
/code/mission/missionparse.cpp: 1005 in parse_cutscenes(mission *)()
999
1000 // add it
1001 pm->cutscenes.push_back(scene);
1002 }
1003
1004 // for reverse compatibility, check that we have a closing tag
>>> CID 1266279: Unchecked return value (CHECKED_RETURN)
>>> Calling "optional_string" without checking return value (as is done elsewhere 1561 out of 1571 times).
1005 optional_string("#end");
1006 }
1007 }
1008
1009 void parse_plot_info(mission *pm)
1010 {
TagsNo tags attached.
Attached Files

Notes
(0016756)
Echelon9   
2015-06-25 00:17   
(Last edited: 2015-06-25 00:18)
Whilst resolving the unchecked return value is easy, Coverity may have come across an inconsistency between the code and documentation here.

Based on my reading of: http://www.hard-light.net/wiki/index.php/Cutscene_flags

1. If a #Cutscenes is found in a mission file, then
2. There may be ANY number of a list corresponding to the MOVIE_* #defines, and for each
3. There may be ONE +formula: tag,

after which,

4. There MUST be an #end tag.

This is further confirmed by scanning the source code, from which it is apparent there is only one other use of #end with optional_string().

grep -r --exclude-dir=.git --exclude-dir=documentation "string(\"#end"

So the correct fix here could be a refactor to align with the documentation. Provided missions were (manually) written with the documented feature, there should be no breakage of pre-exiting missions.

(0016757)
Echelon9   
2015-06-25 00:23   
Proposed PR on GitHub: https://github.com/scp-fs2open/fs2open.github.com/pull/212
(0016758)
Echelon9   
2015-06-26 07:11   
Fixed in master merged commit: https://github.com/scp-fs2open/fs2open.github.com/commit/464d49d182912785616115f3360df33e267a7d37
(0016760)
Goober5000   
2015-06-29 17:02   
I must point out that there exist missions written with #end and missions written without it. That's why when I refactored the routine in question, I specifically commented that the line was for reverse compatibility. Otherwise I would have used required_string().

The wiki documentation is not always synchronized with what FRED emits and accepts.

All that said, the change shouldn't break anything. If a mission is encountered without the #end tag, the modified code flow should still handle it. But it seems less clear than the original commit, and it no longer has the clarification comment for reverse compatibility.

Issue History
2015-01-25 19:45chief1983New Issue
2015-06-23 05:36Echelon9Assigned To => Echelon9
2015-06-23 05:36Echelon9Statusnew => assigned
2015-06-25 00:17Echelon9Note Added: 0016756
2015-06-25 00:18Echelon9Note Edited: 0016756bug_revision_view_page.php?bugnote_id=16756#r1052
2015-06-25 00:23Echelon9Note Added: 0016757
2015-06-25 00:23Echelon9Statusassigned => code review
2015-06-26 07:11Echelon9Note Added: 0016758
2015-06-26 07:11Echelon9Statuscode review => resolved
2015-06-26 07:11Echelon9Fixed in Version => 3.7.3
2015-06-26 07:11Echelon9Resolutionopen => fixed
2015-06-29 17:02Goober5000Note Added: 0016760