|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0003137||FSSCP||gameplay||public||2015-01-25 19:45||2015-06-29 17:02|
|Target Version||3.7.4||Fixed in Version||3.7.3|
|Summary||0003137: 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
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).
1009 void parse_plot_info(mission *pm)
|Tags||No tags attached.|
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,
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.
|Proposed PR on GitHub: https://github.com/scp-fs2open/fs2open.github.com/pull/212|
|Fixed in master merged commit: https://github.com/scp-fs2open/fs2open.github.com/commit/464d49d182912785616115f3360df33e267a7d37|
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.
|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|