2019-10-19 12:45 EDT

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0003137FSSCPgameplaypublic2015-06-29 17:02
Assigned ToEchelon9 
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 *)()
1000 // add it
1001 pm->cutscenes.push_back(scene);
1002 }
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 }
1009 void parse_plot_info(mission *pm)
1010 {
TagsNo tags attached.
Attached Files




Echelon9 (developer)

Last edited: 2015-06-25 00:18

View 2 revisions

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.


Echelon9 (developer)

Proposed PR on GitHub: https://github.com/scp-fs2open/fs2open.github.com/pull/212


Echelon9 (developer)

Fixed in master merged commit: https://github.com/scp-fs2open/fs2open.github.com/commit/464d49d182912785616115f3360df33e267a7d37


Goober5000 (administrator)

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
Date Modified Username Field Change
2015-01-25 19:45 chief1983 New Issue
2015-06-23 05:36 Echelon9 Assigned To => Echelon9
2015-06-23 05:36 Echelon9 Status new => assigned
2015-06-25 00:17 Echelon9 Note Added: 0016756
2015-06-25 00:18 Echelon9 Note Edited: 0016756 View Revisions
2015-06-25 00:23 Echelon9 Note Added: 0016757
2015-06-25 00:23 Echelon9 Status assigned => code review
2015-06-26 07:11 Echelon9 Note Added: 0016758
2015-06-26 07:11 Echelon9 Status code review => resolved
2015-06-26 07:11 Echelon9 Fixed in Version => 3.7.3
2015-06-26 07:11 Echelon9 Resolution open => fixed
2015-06-29 17:02 Goober5000 Note Added: 0016760
+Issue History