Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve TeamsInfoRec::Add_Team function #638

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

xezon
Copy link
Contributor

@xezon xezon commented Feb 26, 2022

Static analyzer still warns about m_teams[m_numTeams++].Init(team); buffer overrun but algorithm looks ok to me.

@xezon xezon added the fixing label Feb 26, 2022
@xezon xezon force-pushed the fix-errors-07-addteam branch 2 times, most recently from e02f9f7 to 8894060 Compare June 16, 2022 08:01
@@ -30,36 +30,31 @@ TeamsInfoRec::~TeamsInfoRec()
*/
void TeamsInfoRec::Add_Team(const Dict *team)
{
captainslog_dbgassert(m_numTeams < 2048, "%d teams have been allocated (so far). This seems excessive.", m_numTeams);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As seen in world builder

// If we have too many teams for our current allocation to handle, reallocate more.
if (m_numTeams >= m_numTeamsAllocated) {
TeamsInfo *ti = new TeamsInfo[m_numTeamsAllocated + TEAMINFO_GROWTH_STEP];
const int allocsz = m_numTeamsAllocated + TEAMINFO_GROWTH_STEP;
captainslog_assert(m_numTeams < allocsz);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this assert for good measure.


// Copy existing data across to new array.
int i;
for (i = 0; i < m_numTeams; ++i) {
int i = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helped avoid complaint from static analyzer I think.

m_teams = ti;
m_numTeamsAllocated += TEAMINFO_GROWTH_STEP;
}

m_teams[m_numTeams].Clear();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the test are removed, because it already happens inside the Init function called below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant