1

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.

9
  • There could be a gazillion reasons, not necessarily related to sets and their iterators per se. Perhaps your string was already destroyed together with the entire tags set. Try inserting diagnostic output, or looking at the variables in the debugger. Aug 3, 2014 at 2:57
  • @n.m., but tags is a field of a UpgradeType instance which is never destroyed until the game is done. As far as I know, the members of the vector won't be destroyed until the vector is.
    – Kat
    Aug 3, 2014 at 3:00
  • The fact is thst the program crashes. This means some of your asumptions about its behaviour are not valid. Validate everything with independent checks. Aug 3, 2014 at 3:08
  • You have correct copy-constructor and copy-assignment operators in your classes? Aug 3, 2014 at 3:09
  • 1
    What is the return type of getTags() ? If it returns a container by value then that loop will go horribly wrong.
    – M.M
    Aug 3, 2014 at 3:38

1 Answer 1

1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.