View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003137 | FSSCP | gameplay | public | 2015-01-26 00:45 | 2015-06-29 21:02 |
| Reporter | chief1983 | Assigned To | Echelon9 | ||
| Priority | normal | Severity | minor | Reproducibility | N/A |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.7.1 | ||||
| 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 *)() 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 { | ||||
| Tags | No tags attached. | ||||
|
|
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. |
|
|
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. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2015-01-26 00:45 | chief1983 | New Issue | |
| 2015-06-23 09:36 | Echelon9 | Assigned To | => Echelon9 |
| 2015-06-23 09:36 | Echelon9 | Status | new => assigned |
| 2015-06-25 04:17 | Echelon9 | Note Added: 0016756 | |
| 2015-06-25 04:18 | Echelon9 | Note Edited: 0016756 | |
| 2015-06-25 04:23 | Echelon9 | Note Added: 0016757 | |
| 2015-06-25 04:23 | Echelon9 | Status | assigned => code review |
| 2015-06-26 11:11 | Echelon9 | Note Added: 0016758 | |
| 2015-06-26 11:11 | Echelon9 | Status | code review => resolved |
| 2015-06-26 11:11 | Echelon9 | Fixed in Version | => 3.7.3 |
| 2015-06-26 11:11 | Echelon9 | Resolution | open => fixed |
| 2015-06-29 21:02 | Goober5000 | Note Added: 0016760 |
