CODEWITHSUNDEEP
c++segmentation-fault
Kat
Kat

Reputation: 4695

Dereferencing a set iterator causes a seg fault

I'm trying to determine why the following code is throwing a segfault on line 10 (where we dereference upgradeIter).

bool UpgradeType::isAffected(const UnitType *unitType) const{
    if(std::find(effects.begin(), effects.end(), unitType)!=effects.end()) return true;

    // Check if the unit has any of the affected tags
    std::set<string>::iterator upgradeIter;
    for(upgradeIter = tags.begin(); upgradeIter != tags.end(); ++upgradeIter) {
        std::set<string>::iterator unitIter;
        for(unitIter = unitType->getTags().begin(); unitIter != unitType->getTags().end(); ++unitIter) {
            string unitTag = *unitIter;
            string upgradeTag = *upgradeIter;
            if(unitTag == upgradeTag) return true;
        }
    }

    return false;
}

The context is that UpgradeType has "tags" (just a set of strings). Units also have tags. If a unit shares at least one tag with the upgrade, then the unit is affected by the upgrade.

I don't see any reason why the mentioned line would crash. It seems to me that there is no circumstances under which the iterator could be invalid.

In other parts of the code that display the contents of tags (used in very similar ways), the output is as expected.

EDIT: I've just found out that unitType->getTags().size() is 0. So I don't understand why the body of the for loop is even executed. unitIter != unitType->getTags().end(), however, is evaluating to true. This seems off.

Upvotes: 1

Views: 577

Answers (1)

Kat
Kat

Reputation: 4695

I managed to find a solution to this with the help of Yggdrasil on this site (which also means that Matt McNabb in the question's comments was correct). Quoting his post below:

As someone more or less mentioned on stackoverflow: Change getTags() to return a reference, not a value/copy.

const set &getTags() const {return tags;}

Be aware that the return type is const, so use a const iterator.

Not sure if that's all, but you don't want a (deep) copy there, for sure. The iterator gets out of bounds because you check against the end of a different set. Every call to getTags() gets its own copy.

Upvotes: 1

Related Questions