joto / osmium

C++/Javascript framework for working with OSM files.
http://wiki.openstreetmap.org/wiki/Osmium
GNU General Public License v3.0
123 stars 31 forks source link

Problem with 64-bit ID #72

Closed alex85k closed 11 years ago

alex85k commented 11 years ago

When running on Windows I stumbled upon the problem: node IDs greater than 2^31 were not stored correctly. After debugging I have found the source of problem:

        Object& id(const char* id) {
            m_id = atol(id);
            return *this;
        }

It seems that on Unix systems atol returns 64-bit integer correctly but not on Windows. Can you please use atoll or some #ifdef to determine that osm_object_id_t is int64_t ?

joto commented 11 years ago

I have looked around a bit and could not find a way to make this portable and fast. There is boost::lexical_cast, but that uses streams which adds an enormous amount of overhead for something as simple as parsing an int.

There are two calls to atoi(), three to atol(), and two to atoll() in Osmium. I guess we have to clear all of that up. Patches welcome.

springmeyer commented 11 years ago

boost spirit is another option: https://github.com/mapnik/mapnik/blob/master/src/conversions.cpp#L114-L118

alex85k commented 11 years ago

Maybe #define in types header and #ifdef in other places will be sufficient? I am afraid spirit or lexical_cast may slow down the processing significantly.

joto commented 11 years ago

I have moved the ato* calls into types.hpp and used atoll() instead of atol(). This should resolve this issue. See 32328816314ba5076acd5206ff86f54db20d9fff. I am not sure this is the best solution, but we can improve on this now that everything is together in one place.

alex85k commented 11 years ago

I like this solution, it is simple enough and located near all other type definitions.

alex85k commented 11 years ago

Close the ticket then?