From eb1a6944877e5147813a10a887c15b209e2640ef Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 12:19:47 +0100 Subject: [PATCH 1/8] Fix valid-in-python2 but invalid-in-python3 try-except syntax --- tests/regression-test.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/regression-test.py b/tests/regression-test.py index fca7b3348..fd95a2254 100755 --- a/tests/regression-test.py +++ b/tests/regression-test.py @@ -336,7 +336,7 @@ def dbConnect(self): self.conn=psycopg2.connect("dbname='osm2pgsql-test'") self.conn.autocommit = True self.cur = self.conn.cursor() - except Exception, e: + except Exception as e: print "I am unable to connect to the database." + e def dbClose(self): @@ -352,7 +352,7 @@ def executeStatements(self, seq): try: self.cur.execute(sql_test_statements[i][2]) res = self.cur.fetchall() - except Exception, e: + except Exception as e: self.assertEqual(0, 1, str(sql_test_statements[i][0]) + ": Failed to execute " + sql_test_statements[i][1] + " (" + sql_test_statements[i][2] + ") {" + str(self.parameters) +"}") if (res == None): @@ -507,13 +507,13 @@ def setupDB(): try: gen_conn=psycopg2.connect("dbname='template1'") gen_conn.autocommit = True - except Exception, e: + except Exception as e: print "I am unable to connect to the database." exit(1) try: gen_cur = gen_conn.cursor() - except Exception, e: + except Exception as e: gen_conn.close() print "I am unable to connect to the database." exit(1) @@ -521,7 +521,7 @@ def setupDB(): try: gen_cur.execute("""DROP DATABASE IF EXISTS \"osm2pgsql-test\"""") gen_cur.execute("""CREATE DATABASE \"osm2pgsql-test\" WITH ENCODING 'UTF8'""") - except Exception, e: + except Exception as e: print "Failed to create osm2pgsql-test db" + e.pgerror exit(1); finally: @@ -531,13 +531,13 @@ def setupDB(): try: test_conn=psycopg2.connect("dbname='osm2pgsql-test'") test_conn.autocommit = True - except Exception, e: + except Exception as e: print "I am unable to connect to the database." + e exit(1) try: test_cur = test_conn.cursor() - except Exception, e: + except Exception as e: print "I am unable to connect to the database." + e gen_conn.close() exit(1) @@ -558,7 +558,7 @@ def setupDB(): print " sudo /bin/chown postgres.postgres tmp/psql-tablespace" print " psql -c \"CREATE TABLESPACE tablespacetest LOCATION '/tmp/psql-tablespace'\" postgres" exit(77) - except Exception, e: + except Exception as e: print "Failed to create directory for tablespace" + str(e) # Check for postgis @@ -578,7 +578,7 @@ def setupDB(): # Check for hstore support try: test_cur.execute("""CREATE EXTENSION hstore;""") - except Exception, e: + except Exception as e: hst = findContribSql('hstore.sql') pgscript = open(hst).read() test_cur.execute(pgscript) @@ -593,7 +593,7 @@ def tearDownDB(): gen_conn=psycopg2.connect("dbname='template1'") gen_conn.autocommit = True gen_cur = gen_conn.cursor() - except Exception, e: + except Exception as e: print "I am unable to connect to the database." exit(1) @@ -601,7 +601,7 @@ def tearDownDB(): gen_cur.execute("""DROP DATABASE IF EXISTS \"osm2pgsql-test\"""") if (created_tablespace == 1): gen_cur.execute("""DROP TABLESPACE IF EXISTS \"tablespacetest\"""") - except Exception, e: + except Exception as e: print "Failed to clean up osm2pgsql-test db" + e.pgerror exit(1); From b5590c2bb897e72ef3ac431db0a75cc02df80607 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 12:27:15 +0100 Subject: [PATCH 2/8] Include required iostream for std::cerr; was missing and causing test fails --- tests/test-pgsql-escape.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test-pgsql-escape.cpp b/tests/test-pgsql-escape.cpp index 0fcd24e2c..477db7ad5 100644 --- a/tests/test-pgsql-escape.cpp +++ b/tests/test-pgsql-escape.cpp @@ -1,3 +1,4 @@ +#include #include "pgsql.hpp" void test_escape(const char *in, const char *out) { From dc52597b7ad69d4e046e5f15d6eba684775d4251 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 12:56:52 +0100 Subject: [PATCH 3/8] Space before print formatters required otherwise this is parsed as C++11 literal --- parse-o5m.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse-o5m.cpp b/parse-o5m.cpp index b6fc9de96..5d242a0d9 100644 --- a/parse-o5m.cpp +++ b/parse-o5m.cpp @@ -402,7 +402,7 @@ static void read_close() { return; fd= read_infop->fd; if(loglevel>=1) { /* verbose */ - fprintf(stderr,"osm2pgsql: Number of bytes read: %"PRIu64"\n", + fprintf(stderr,"osm2pgsql: Number of bytes read: %" PRIu64 "\n", read_infop->read__counter); } if(loglevel>=2) { From 374e8468745b889b8e35d9f402b4970656e8cf1a Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 12:59:12 +0100 Subject: [PATCH 4/8] ';' outside function is C++11 extension --- middle-pgsql.cpp | 2 +- output-pgsql.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/middle-pgsql.cpp b/middle-pgsql.cpp index 62eeda791..c38346c0f 100644 --- a/middle-pgsql.cpp +++ b/middle-pgsql.cpp @@ -1263,7 +1263,7 @@ struct pthread_thunk { extern "C" void *pthread_middle_pgsql_stop_one(void *arg) { pthread_thunk *thunk = static_cast(arg); return thunk->obj->pgsql_stop_one(thunk->ptr); -}; +} } // anonymous namespace void middle_pgsql_t::stop(void) diff --git a/output-pgsql.cpp b/output-pgsql.cpp index 8add8a0ff..5ec95802b 100644 --- a/output-pgsql.cpp +++ b/output-pgsql.cpp @@ -252,7 +252,7 @@ extern "C" void *pthread_output_pgsql_stop_one(void *arg) { pthread_thunk *thunk = static_cast(arg); thunk->ptr->stop(); return NULL; -}; +} } // anonymous namespace void output_pgsql_t::enqueue_ways(pending_queue_t &job_queue, osmid_t id, size_t output_id, size_t& added) { From 2ff793cdb7afcd4103a284ca26c1e7b3d4bc2849 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 14:57:11 +0100 Subject: [PATCH 5/8] Modernize geometry builder: std::sort instead of ::qsort, rip out manual memory management, catch by const ref --- geometry-builder.cpp | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/geometry-builder.cpp b/geometry-builder.cpp index 560366570..a7370b329 100644 --- a/geometry-builder.cpp +++ b/geometry-builder.cpp @@ -21,6 +21,7 @@ */ #include +#include #include #include #include @@ -81,15 +82,12 @@ struct polygondata unsigned containedbyid; }; -int polygondata_comparearea(const void* vp1, const void* vp2) -{ - const polygondata* p1 = (const polygondata*)vp1; - const polygondata* p2 = (const polygondata*)vp2; +struct polygondata_comparearea { + bool operator()(const polygondata& lhs, const polygondata& rhs) { + return lhs.area > rhs.area; + } +}; - if (p1->area == p2->area) return 0; - if (p1->area > p2->area) return -1; - return 1; -} } // anonymous namespace geometry_builder::maybe_wkt_t geometry_builder::get_wkt_simple(const osmNode *nodes, int count, int polygon) const @@ -130,12 +128,12 @@ geometry_builder::maybe_wkt_t geometry_builder::get_wkt_simple(const osmNode *no wkt->geom = WKTWriter().write(geom.get()); return wkt; } - catch (std::bad_alloc&) + catch (const std::bad_alloc&) { std::cerr << std::endl << "Exception caught processing way. You are likelly running out of memory." << std::endl; std::cerr << "Try in slim mode, using -s parameter." << std::endl; } - catch (std::runtime_error& e) + catch (const std::runtime_error& e) { //std::cerr << std::endl << "Exception caught processing way: " << e.what() << std::endl; } @@ -247,12 +245,12 @@ geometry_builder::maybe_wkts_t geometry_builder::get_wkt_split(const osmNode *no } } } - catch (std::bad_alloc&) + catch (const std::bad_alloc&) { std::cerr << std::endl << "Exception caught processing way. You are likely running out of memory." << std::endl; std::cerr << "Try in slim mode, using -s parameter." << std::endl; } - catch (std::runtime_error& e) + catch (const std::runtime_error& e) { //std::cerr << std::endl << "Exception caught processing way: " << e.what() << std::endl; } @@ -363,7 +361,7 @@ geometry_builder::maybe_wkts_t geometry_builder::build_polygons(const osmNode * WKTWriter writer; // Procces ways into lines or simple polygon list - polygondata* polys = new polygondata[merged->size()]; + std::vector polys(merged->size()); unsigned totalpolys = 0; for (unsigned i=0 ;i < merged->size(); ++i) @@ -387,7 +385,7 @@ geometry_builder::maybe_wkts_t geometry_builder::build_polygons(const osmNode * if (totalpolys) { - qsort(polys, totalpolys, sizeof(polygondata), polygondata_comparearea); + std::sort(polys.begin(), polys.begin() + totalpolys, polygondata_comparearea()); unsigned toplevelpolygons = 0; int istoplevelafterall; @@ -509,9 +507,8 @@ geometry_builder::maybe_wkts_t geometry_builder::build_polygons(const osmNode * { delete(polys[i].polygon); } - delete[](polys); }//TODO: don't show in message id when osm_id == -1 - catch (std::exception& e) + catch (const std::exception& e) { std::cerr << std::endl << "Standard exception processing way_id="<< osm_id << ": " << e.what() << std::endl; } @@ -556,7 +553,7 @@ geometry_builder::maybe_wkt_t geometry_builder::build_multilines(const osmNode * wkt->geom = writer.write(mline.get()); wkt->area = 0; }//TODO: don't show in message id when osm_id == -1 - catch (std::exception& e) + catch (const std::exception& e) { std::cerr << std::endl << "Standard exception processing way_id="<< osm_id << ": " << e.what() << std::endl; } @@ -604,7 +601,7 @@ geometry_builder::maybe_wkts_t geometry_builder::build_both(const osmNode * cons WKTWriter writer; // Procces ways into lines or simple polygon list - polygondata* polys = new polygondata[merged->size()]; + std::vector polys(merged->size()); unsigned totalpolys = 0; for (unsigned i=0 ;i < merged->size(); ++i) @@ -657,7 +654,7 @@ geometry_builder::maybe_wkts_t geometry_builder::build_both(const osmNode * cons if (totalpolys) { - qsort(polys, totalpolys, sizeof(polygondata), polygondata_comparearea); + std::sort(polys.begin(), polys.begin() + totalpolys, polygondata_comparearea()); unsigned toplevelpolygons = 0; int istoplevelafterall; @@ -779,9 +776,8 @@ geometry_builder::maybe_wkts_t geometry_builder::build_both(const osmNode * cons { delete(polys[i].polygon); } - delete[](polys); }//TODO: don't show in message id when osm_id == -1 - catch (std::exception& e) + catch (const std::exception& e) { std::cerr << std::endl << "Standard exception processing relation id="<< osm_id << ": " << e.what() << std::endl; } From 4a5fc188e70a24c9772765c5acbb324e18b07be0 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 15:15:26 +0100 Subject: [PATCH 6/8] Globally: catch by const ref --- osm2pgsql.cpp | 2 +- output-pgsql.cpp | 2 +- tests/test-expire-tiles.cpp | 2 +- tests/test-parse-options.cpp | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osm2pgsql.cpp b/osm2pgsql.cpp index 59a191585..fadb41ca8 100644 --- a/osm2pgsql.cpp +++ b/osm2pgsql.cpp @@ -119,7 +119,7 @@ int main(int argc, char *argv[]) return 0; }//something went wrong along the way - catch(std::runtime_error& e) + catch(const std::runtime_error& e) { fprintf(stderr, "Osm2pgsql failed due to ERROR: %s\n", e.what()); exit(EXIT_FAILURE); diff --git a/output-pgsql.cpp b/output-pgsql.cpp index 5ec95802b..473038135 100644 --- a/output-pgsql.cpp +++ b/output-pgsql.cpp @@ -666,7 +666,7 @@ output_pgsql_t::output_pgsql_t(const middle_query_t* mid_, const options_t &opti try { m_tagtransform.reset(new tagtransform(&m_options)); } - catch(std::runtime_error& e) { + catch(const std::runtime_error& e) { fprintf(stderr, "%s\n", e.what()); fprintf(stderr, "Error: Failed to initialise tag processing.\n"); util::exit_nicely(); diff --git a/tests/test-expire-tiles.cpp b/tests/test-expire-tiles.cpp index 62f8fe515..4bc0b43c9 100644 --- a/tests/test-expire-tiles.cpp +++ b/tests/test-expire-tiles.cpp @@ -19,7 +19,7 @@ void run_test(const char* test_name, void (*testfunc)()) fprintf(stderr, "%s\n", test_name); testfunc(); } - catch(std::exception& e) + catch(const std::exception& e) { fprintf(stderr, "%s\n", e.what()); fprintf(stderr, "FAIL\n"); diff --git a/tests/test-parse-options.cpp b/tests/test-parse-options.cpp index 8cc57ee91..bfe910a88 100644 --- a/tests/test-parse-options.cpp +++ b/tests/test-parse-options.cpp @@ -24,7 +24,7 @@ void run_test(const char* test_name, void (*testfunc)()) fprintf(stderr, "%s\n", test_name); testfunc(); } - catch(std::exception& e) + catch(const std::exception& e) { fprintf(stderr, "%s\n", e.what()); fprintf(stderr, "FAIL\n"); @@ -40,7 +40,7 @@ void parse_fail(const int argc, const char* argv[], const std::string& fail_mess options_t options = options_t::parse(argc, const_cast(argv)); throw std::logic_error((boost::format("Expected '%1%'") % fail_message).str()); } - catch(std::runtime_error& e) + catch(const std::runtime_error& e) { if(!alg::icontains(e.what(), fail_message)) throw std::logic_error((boost::format("Expected '%1%' but instead got '%2%'") % fail_message % e.what()).str()); @@ -125,7 +125,7 @@ void test_outputs() out = outs.front().get(); throw std::logic_error("Expected 'not recognised'"); } - catch(std::runtime_error& e) + catch(const std::runtime_error& e) { if(!alg::icontains(e.what(), "not recognised")) throw std::logic_error((boost::format("Expected 'not recognised' but instead got '%2%'") % e.what()).str()); From f347bf88967ffc5d420bd284a3ded2466b7950da Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Sat, 21 Feb 2015 15:45:21 +0100 Subject: [PATCH 7/8] Incrementally rip out manually managed memory --- output-gazetteer.cpp | 37 +++++++++++++------------------------ output-pgsql.cpp | 38 +++++++++++++------------------------- 2 files changed, 26 insertions(+), 49 deletions(-) diff --git a/output-gazetteer.cpp b/output-gazetteer.cpp index 689e0fb8f..8789d1277 100644 --- a/output-gazetteer.cpp +++ b/output-gazetteer.cpp @@ -692,22 +692,20 @@ int output_gazetteer_t::process_way(osmid_t id, osmid_t *ndv, int ndc, /* Are we interested in this item? */ if (places.has_data()) { - struct osmNode *nodev; int nodec; + assert(ndc > 0 && "no nodes to proccess"); + /* Fetch the node details */ - nodev = (struct osmNode *)malloc(ndc * sizeof(struct osmNode)); - nodec = m_mid->nodes_get_list(nodev, ndv, ndc); + std::vector nodev(ndc); + nodec = m_mid->nodes_get_list(&nodev[0], ndv, ndc); /* Get the geometry of the object */ - geometry_builder::maybe_wkt_t wkt = builder.get_wkt_simple(nodev, nodec, 1); + geometry_builder::maybe_wkt_t wkt = builder.get_wkt_simple(&nodev[0], nodec, 1); if (wkt) { places.copy_out('W', id, wkt->geom, buffer); flush_place_buffer(); } - - /* Free the nodes */ - free(nodev); } return 0; @@ -742,7 +740,7 @@ int output_gazetteer_t::process_relation(osmid_t id, struct member *members, return 0; /* get the boundary path (ways) */ - osmid_t *xid2 = new osmid_t[member_count+1]; + std::vector xid2(member_count+1); int count = 0; for (int i=0; iways_get_list(xid2, count, xid, xtags, xnodes, xcount); + std::vector xcount(count + 1); + std::vector xtags(count + 1); + std::vector xnodes(count + 1); + std::vector xid(count + 1); + count = m_mid->ways_get_list(&xid2[0], count, &xid[0], &xtags[0], &xnodes[0], &xcount[0]); xnodes[count] = NULL; xcount[count] = 0; if (cmp_waterway) { - geometry_builder::maybe_wkts_t wkts = builder.build_both(xnodes, xcount, 1, 1, 1000000, id); + geometry_builder::maybe_wkts_t wkts = builder.build_both(&xnodes[0], &xcount[0], 1, 1, 1000000, id); for (geometry_builder::wkt_itr wkt = wkts->begin(); wkt != wkts->end(); ++wkt) { if (boost::starts_with(wkt->geom, "POLYGON") || boost::starts_with(wkt->geom, "MULTIPOLYGON")) { @@ -784,7 +780,7 @@ int output_gazetteer_t::process_relation(osmid_t id, struct member *members, } } else { /* waterways result in multilinestrings */ - geometry_builder::maybe_wkt_t wkt = builder.build_multilines(xnodes, xcount, id); + geometry_builder::maybe_wkt_t wkt = builder.build_multilines(&xnodes[0], &xcount[0], id); if ((wkt->geom).length() > 0) { places.copy_out('R', id, wkt->geom, buffer); flush_place_buffer(); @@ -794,15 +790,8 @@ int output_gazetteer_t::process_relation(osmid_t id, struct member *members, for (int i=0; i members_superseeded(member_count); //if its a route relation make_boundary and make_polygon will be false otherwise one or the other will be true - if (m_tagtransform->filter_rel_member_tags(rel_tags, member_count, xtags, xrole, members_superseeded, &make_boundary, &make_polygon, &roads, m_export_list.get())) { - free(members_superseeded); + if (m_tagtransform->filter_rel_member_tags(rel_tags, member_count, xtags, xrole, &members_superseeded[0], &make_boundary, &make_polygon, &roads, m_export_list.get())) { return 0; } @@ -180,7 +178,6 @@ int output_pgsql_t::pgsql_out_relation(osmid_t id, struct keyval *rel_tags, int geometry_builder::maybe_wkts_t wkts = builder.build_both(xnodes, xcount, make_polygon, m_options.enable_multi, split_at, id); if (!wkts->size()) { - free(members_superseeded); return 0; } @@ -217,8 +214,6 @@ int output_pgsql_t::pgsql_out_relation(osmid_t id, struct keyval *rel_tags, int } } - free(members_superseeded); - // If the tag transform said the polygon looked like a boundary we want to make that as well // If we are making a boundary then also try adding any relations which form complete rings // The linear variants will have already been processed above @@ -424,10 +419,9 @@ int output_pgsql_t::way_add(osmid_t id, osmid_t *nds, int nd_count, struct keyva if( !polygon && !filter ) { /* Get actual node data and generate output */ - struct osmNode *nodes = (struct osmNode *)malloc( sizeof(struct osmNode) * nd_count ); - int count = m_mid->nodes_get_list( nodes, nds, nd_count ); - pgsql_out_way(id, tags, nodes, count, 0); - free(nodes); + std::vector nodes(nd_count); + int count = m_mid->nodes_get_list(&nodes[0], nds, nd_count); + pgsql_out_way(id, tags, &nodes[0], count, 0); } return 0; } @@ -446,11 +440,11 @@ int output_pgsql_t::pgsql_process_relation(osmid_t id, const struct member *memb return 1; } - osmid_t *xid2 = (osmid_t *)malloc( (member_count+1) * sizeof(osmid_t) ); - const char **xrole = (const char **)malloc( (member_count+1) * sizeof(const char *) ); - int *xcount = (int *)malloc( (member_count+1) * sizeof(int) ); - keyval *xtags = new keyval[member_count+1]; - struct osmNode **xnodes = (struct osmNode **)malloc( (member_count+1) * sizeof(struct osmNode*) ); + std::vector xid2(member_count+1); + std::vector xrole(member_count+1); + std::vector xcount(member_count+1); + std::vector xtags(member_count+1); + std::vector xnodes(member_count+1); count = 0; for( i=0; iways_get_list(xid2, count, xid, xtags, xnodes, xcount); + std::vector xid(count + 1); + count2 = m_mid->ways_get_list(&xid2[0], count, &xid[0], &xtags[0], &xnodes[0], &xcount[0]); int polygon = 0, roads = 0;; for (i = 0; i < count2; i++) { @@ -489,7 +483,7 @@ int output_pgsql_t::pgsql_process_relation(osmid_t id, const struct member *memb xrole[count2] = NULL; /* At some point we might want to consider storing the retrieved data in the members, rather than as separate arrays */ - pgsql_out_relation(id, tags, count2, xnodes, xtags, xcount, xid, xrole, pending); + pgsql_out_relation(id, tags, count2, &xnodes[0], &xtags[0], &xcount[0], &xid[0], &xrole[0], pending); for( i=0; i Date: Sat, 21 Feb 2015 16:17:37 +0100 Subject: [PATCH 8/8] No VLAs in C++ (variable length array extension) --- middle-pgsql.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/middle-pgsql.cpp b/middle-pgsql.cpp index c38346c0f..02fba4e13 100644 --- a/middle-pgsql.cpp +++ b/middle-pgsql.cpp @@ -1270,14 +1270,14 @@ void middle_pgsql_t::stop(void) { int i; #ifdef HAVE_PTHREAD - pthread_t threads[num_tables]; + std::vector threads(num_tables); #endif cache.reset(); if (out_options->flat_node_cache_enabled) persistent_cache.reset(); #ifdef HAVE_PTHREAD - pthread_thunk thunks[num_tables]; + std::vector thunks(num_tables); for (i=0; i