CleverRaven/Cataclysm-DDA — pull request #8734 by BevapDin (2014-08-29 09:56:17+00:00)
Title: Fix crash from teleglow

Fixes #8612

The crash is caused by `dis_effect`, specifically with the teleglow disease.

It adds and/or removes diseases from the player object. Adding diseases can lead to a reallocation of the diseases vector (`player::illness`). As the parameter to `dis_effect` (`dis`) is a reference into that vector, it will be invalidated by that reallocation. But `dis_effect` continues to examine that object:
```
if (dis.duration > 6000) {
...
if() p.add_disease("shakes")
}
// -- no "else if", dis.duration is read again!
if (dis.duration > 3000) {
...
if() p.add_disease("hallu")
}
```

`dis_effect` also calls `player::rem_disease` which modifies the illness vector directly, this invalidate the parameter `dis`. Example: two diseases are present: [teleglow, sleep] (in that order), `dis_effect` is called with a reference to the first entry. Removing teleglow will cause the sleep disease to be moved into the first entry of the vector. `dis` now refers to the sleep disease. But this will probably not cause a crash.

In short:

- `dis_effect` can not remove the disease it is currently working on as it would invalidate its parameter (it can remove the disease, but than it must not refer to the parameter again until the function is left).

- `dis_effect` can not add diseases as this would invalidate the parameter with the same consequences as above.

The "laying down" disease for example avoids those problems: it adds the sleep disease and immediately leaves the function. It also does not call `player::rem_disease` but sets `disease::duration` to 1 instead. But anyone working on this code has to remember that trick.

----

To avoid both problems I changed `player::illness` to be a std::list. This allows adding diseases without the danger of reallocation. The iteration in `player::suffer` uses iterators now, which are not affected by the addition of disease to the `std::list`.

Another change: `player::remove_disease` sets the duration to -1, but does not remove the entry in the illness list. The entry is later removed in `player::suffer`. This makes it safe to remove a disease (in `dis_effect`) while still referring to the disease object. The object stays valid until it is removed (along with all regularly gone diseases) in `player::suffer`.