Skip to content

Commit

Permalink
Fix off-by-1 in Group::Remap.
Browse files Browse the repository at this point in the history
This was introduced in bd84bc1 and caused crashes with:
  Assertion failed: hm.v != t->h.v.
  • Loading branch information
whitequark committed Jun 24, 2019
1 parent cb0fdb1 commit 49a7f86
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/group.cpp
Expand Up @@ -718,7 +718,7 @@ hEntity Group::Remap(hEntity in, int copyNumber) {
auto it = remap.find({ in, copyNumber });
if(it == remap.end()) {
std::tie(it, std::ignore) =
remap.insert({ { in, copyNumber }, { (uint32_t)remap.size() } });
remap.insert({ { in, copyNumber }, { (uint32_t)remap.size() + 1 } });
}
return h.entity(it->second.v);
}
Expand Down

5 comments on commit 49a7f86

@phkahler
Copy link
Member

Choose a reason for hiding this comment

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

Any idea if this means files created with the bug are broken? Not a big deal for me, but good to know if so.

@whitequark
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they are.

@whitequark
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phkahler I think it's possible to add automated recovery from this. Let me prototype something.

@phkahler
Copy link
Member

Choose a reason for hiding this comment

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

@whitequark kicking myself. I thought about suggesting you revert that change when the issue was reported due to fear of creating broken files.

@whitequark
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phkahler I added automatic recovery. I think the fallout will be minimal, so we got lucky this time.

Please sign in to comment.