Bug: SegFault caused by invalid string about tilemaker HOT 2 OPEN

D'oh, sorry that you're hitting this. I can offer some context.

In #583, I changed AttributePair to identify the key by a short rather than a std::string. This had the effect of limiting us to 64K unique keys, which is probably plenty.

In #618, I further changed it to use at most 9 bits:

That limits us to at most 512 unique keys.

Except... this has the same bug as #672 did: it's using a signed type. Instead of short : 9, it should be unsigned short : 9. As it stands, the usable range is -256..255, not 0..511, so you can actually only squeeze in 256 keys before hitting issues.

So probably there are at least two things that ought to be done:

1. Use unsigned short : 9 so we get the full range.
2. Consider making AttributeKeyStore assert if it would return an index that would be out-of-bounds (so that future people who hit this issue get a useful error, not a segfault)

And then possibly:

3. Permit more than 512 keys to be used, at the expense of increasing the size of AttributePair. Maybe in your scenario that's not needed?

from tilemaker.

@cldellow Nice. My first thought was that it was an overflow, given the unexpected number, but the bounds (-245) just didn't make sense to me. Too big for 8 bit, too small for 16 bit. The daft part is that when looking at the definition short keyIndex : 9; I did wonder about the syntax (I'm not a C/C++ guy, should have googled it though).

Just to verify this...

1. Because, I know the ID of the element, and that the tag value is branch, which means the tag key will be usage and the keyIndex = -245
2. The keyIndex requested was 267 (based on the two's complement
3. Boom, the tag at index 267 is "usage"
4. Also works for the next attribute which was maxspeed with a index of -244

Feeling pretty daft that I didn't work that one out 🤦

So, with regards to this:

1. I've just rebuilt tilemaker changing keyIndex to an unsigned short, and it looks up the indexes without a worry, even processing the stupid amount of attributes for the entirety of Germany
2. That would be ideal, probably better if somebody other than I does that...
3. "512 attributes ought to be enough for anyone"... Jokes aside, I'll leave that one up to people better at this me. 512 is plenty for my use, I'm not planning on having as many as want to abstract many of the attributes I'm chucking on to the tiles.

from tilemaker.