Source Code Project Mantis - FSSCP
View Issue Details
0002688FSSCPmodular tablespublic2012-07-14 13:062012-07-15 14:46
ReporterMjnMixael 
Assigned Tom_m 
PrioritynormalSeveritycrashReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.6.13 
Target VersionFixed in Version 
Summary0002688: Long credits causes stack overflow crash.
DescriptionIf the total length of credits.tbl + code credits + credits.tbms is too long, FSO will crash when you enter the credits viewer.
Steps To ReproducePlace the attached tbm into data/tables and enter the credits viewer.
Additional InformationFrom IRC discussion with The_E...

<@The_E> MjnMixael: Would you be so kind as to throw it on Mantis, for the attention of whoever coded the credits.tbl modularization?
<@MjnMixael> sure
<@The_E> The crash cause is a stack overflow due to excessive amounts of text
<@MjnMixael> it was m|m, btw
<@The_E> This can be fixed by increasing the default stack size, but that needs a project file update
<@MjnMixael> waaaaaaay out of my knowledge here, but is it possible to split up tbms into separate "stacks" and just display them in order of loading, or does it just not work like that?
<@The_E> ___It does not work that way _
<@The_E> Err
<@The_E> It does not work that way currently, but that would indeed be another possible solution
TagsNo tags attached.
Attached Filespatch credits.patch (8,708) 2012-07-14 16:14
http://scp.indiegames.us/mantis/file_download.php?file_id=1921&type=bug
? fsu_credits-crd.tbm (7,780) 2012-07-14 16:15
http://scp.indiegames.us/mantis/file_download.php?file_id=1922&type=bug

Notes
(0013859)
m_m   
2012-07-14 14:30   
I added a patch which implements the suggested behavior and it seems to work as expected and without any crashes.
(0013860)
MjnMixael   
2012-07-14 15:08   
(Last edited: 2012-07-14 15:37)
The patch fixes the crash and displays all the credits properly.

However it also breaks the following table options, as in they parse but don't have any visible effect.

$Text Scroll Rate:
$Artworks Display Time:
$Artworks Fade Time:

(0013861)
m_m   
2012-07-14 16:27   
I updated the patch with another bugfix where values set in a table got overwritten with the default values when a table didn't contain the options.

Also restored the sample table which I accidentally deleted...
(0013863)
MjnMixael   
2012-07-15 02:05   
I confirm all issues are fixed with the latest patch.
(0013864)
Goober5000   
2012-07-15 02:18   
Why does the patch remove the ability to set a default value for the optional float?
(0013865)
m_m   
2012-07-15 04:16   
Because there is not way to know whether the variable has been set before so the value which got set before gets overwritten every time the function doesn't find the optional tag.
(0013867)
Goober5000   
2012-07-15 14:45   
I see what you mean. It also looks like whoever wrote parse_optional_float wasn't familiar with the parsing functions in general. I've rewritten that section slightly to use the conventional methods. I've also added better minimum values, because we don't want the credits to freeze or crash with a div-0 exception due to entering a value of 0.
(0013868)
Goober5000   
2012-07-15 14:46   
Marking as fixed due to testing, code review, and commit.

Issue History
2012-07-14 13:06MjnMixaelNew Issue
2012-07-14 13:06MjnMixaelFile Added: fsu_credits-crd.tbm
2012-07-14 14:28m_mAssigned To => m_m
2012-07-14 14:28m_mStatusnew => assigned
2012-07-14 14:30m_mNote Added: 0013859
2012-07-14 14:30m_mStatusassigned => code review
2012-07-14 14:30m_mFile Added: credits.patch
2012-07-14 15:08MjnMixaelNote Added: 0013860
2012-07-14 15:37MjnMixaelNote Edited: 0013860bug_revision_view_page.php?bugnote_id=13860#r187
2012-07-14 16:14m_mFile Deleted: fsu_credits-crd.tbm
2012-07-14 16:14m_mFile Deleted: credits.patch
2012-07-14 16:14m_mFile Added: credits.patch
2012-07-14 16:15m_mFile Added: fsu_credits-crd.tbm
2012-07-14 16:27m_mNote Added: 0013861
2012-07-15 02:05MjnMixaelNote Added: 0013863
2012-07-15 02:18Goober5000Note Added: 0013864
2012-07-15 04:16m_mNote Added: 0013865
2012-07-15 14:45Goober5000Note Added: 0013867
2012-07-15 14:45Goober5000Changeset attached => fs2open trunk r9017
2012-07-15 14:46Goober5000Note Added: 0013868
2012-07-15 14:46Goober5000Statuscode review => resolved
2012-07-15 14:46Goober5000Resolutionopen => fixed