From 406092b441b66aedd0b4a4cc1767c76ef1e2b3f0 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Wed, 1 Jun 2016 18:25:26 +0200 Subject: [PATCH 1/5] remove some unused system checks and defines The code that needed these things has all been removed. --- CMakeLists.txt | 4 ---- cmake/config.h.in | 3 --- middle-pgsql.cpp | 13 ------------- 3 files changed, 20 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff18d88cc..32a9f2836 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,9 +66,6 @@ add_definitions( -DFIXED_POINT ) CHECK_INCLUDE_FILES (termios.h HAVE_TERMIOS_H) CHECK_INCLUDE_FILES (libgen.h HAVE_LIBGEN_H) CHECK_INCLUDE_FILES (unistd.h HAVE_UNISTD_H) -CHECK_INCLUDE_FILES (sys/wait.h HAVE_SYS_WAIT_H) -CHECK_INCLUDE_FILES (sys/time.h HAVE_SYS_TIME_H) -CHECK_INCLUDE_FILES (sys/mman.h HAVE_MMAP) if (WIN32) set(HAVE_LIBGEN_H FALSE) @@ -78,7 +75,6 @@ CHECK_FUNCTION_EXISTS(lseek64 HAVE_LSEEK64) CHECK_FUNCTION_EXISTS(posix_fallocate HAVE_POSIX_FALLOCATE) CHECK_FUNCTION_EXISTS(posix_fadvise HAVE_POSIX_FADVISE) CHECK_FUNCTION_EXISTS(sync_file_range HAVE_SYNC_FILE_RANGE) -CHECK_FUNCTION_EXISTS(fork HAVE_FORK) CHECK_TYPE_SIZE("off_t" SIZEOF_OFF_T) diff --git a/cmake/config.h.in b/cmake/config.h.in index 292f0df29..f2478264c 100644 --- a/cmake/config.h.in +++ b/cmake/config.h.in @@ -1,4 +1,3 @@ -#cmakedefine HAVE_FORK 1 #cmakedefine HAVE_LSEEK64 1 #cmakedefine HAVE_LUA 1 #cmakedefine HAVE_POSIX_FADVISE 1 @@ -6,8 +5,6 @@ #cmakedefine HAVE_SYNC_FILE_RANGE 1 #cmakedefine HAVE_TERMIOS_H 1 #cmakedefine HAVE_LIBGEN_H 1 -#cmakedefine HAVE_SYS_WAIT_H 1 -#cmakedefine HAVE_UNISTD_H 1 #cmakedefine SIZEOF_OFF_T ${SIZEOF_OFF_T} #ifdef _MSC_VER diff --git a/middle-pgsql.cpp b/middle-pgsql.cpp index e9304e465..1fc5c0367 100644 --- a/middle-pgsql.cpp +++ b/middle-pgsql.cpp @@ -8,19 +8,6 @@ #include "config.h" -#ifdef HAVE_SYS_WAIT_H -#include -#endif - -#ifdef HAVE_MMAP -#include -#ifndef MAP_ANONYMOUS -#ifdef MAP_ANON -#define MAP_ANONYMOUS MAP_ANON -#endif -#endif -#endif - #ifdef _WIN32 using namespace std; #endif From 13def06f1f06d18fbfbd5020c5d3f3c5c85fd221 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Wed, 1 Jun 2016 18:35:17 +0200 Subject: [PATCH 2/5] replace C casts with C++ casts Those are more strict in the conversions they allow, and are easier to search for. --- middle-pgsql.cpp | 24 ++++++++++++------------ pgsql.cpp | 4 ++-- sprompt.cpp | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/middle-pgsql.cpp b/middle-pgsql.cpp index 1fc5c0367..b176ec4ff 100644 --- a/middle-pgsql.cpp +++ b/middle-pgsql.cpp @@ -76,7 +76,7 @@ char *pgsql_store_nodes(const idlist_t &nds) { if( buflen <= nds.size() * 10 ) { buflen = ((nds.size() * 10) | 4095) + 1; // Round up to next page */ - buffer = (char *)realloc( buffer, buflen ); + buffer = static_cast(realloc( buffer, buflen )); } _restart: @@ -92,7 +92,7 @@ char *pgsql_store_nodes(const idlist_t &nds) { if( (size_t) (ptr-buffer) > (buflen-20) ) // Almost overflowed? */ { buflen <<= 1; - buffer = (char *)realloc( buffer, buflen ); + buffer = static_cast(realloc( buffer, buflen )); goto _restart; } @@ -164,7 +164,7 @@ const char *pgsql_store_tags(const taglist_t &tags, bool escape) if( buflen <= countlist * 24 ) // LE so 0 always matches */ { buflen = ((countlist * 24) | 4095) + 1; // Round up to next page */ - buffer = (char *)realloc( buffer, buflen ); + buffer = static_cast(realloc( buffer, buflen )); } _restart: @@ -178,7 +178,7 @@ const char *pgsql_store_tags(const taglist_t &tags, bool escape) if( (ptr+maxlen-buffer) > (buflen-20) ) // Almost overflowed? */ { buflen <<= 1; - buffer = (char *)realloc( buffer, buflen ); + buffer = static_cast(realloc( buffer, buflen )); goto _restart; } @@ -297,7 +297,7 @@ void middle_pgsql_t::local_nodes_set(const osmid_t& id, const double& lat, { const char *tag_buf = pgsql_store_tags(tags,1); int length = strlen(tag_buf) + 64; - char *buffer = (char *)alloca( length ); + char *buffer = static_cast(alloca( length )); #ifdef FIXED_POINT ramNode n(lon, lat); if (snprintf(buffer, length, "%" PRIdOSMID "\t%d\t%d\t%s\n", @@ -313,7 +313,7 @@ void middle_pgsql_t::local_nodes_set(const osmid_t& id, const double& lat, } else { // Four params: id, lat, lon, tags */ const char *paramValues[4]; - char *buffer = (char *)alloca(64); + char *buffer = static_cast(alloca(64)); char *ptr = buffer; paramValues[0] = ptr; ptr += sprintf( ptr, "%" PRIdOSMID, id ) + 1; @@ -340,7 +340,7 @@ size_t middle_pgsql_t::local_nodes_get_list(nodelist_t &out, const idlist_t nds) char tmp[16]; - char *tmp2 = (char *)malloc(sizeof(char) * nds.size() * 16); + char *tmp2 = static_cast(malloc(sizeof(char) * nds.size() * 16)); if (tmp2 == nullptr) return 0; //failed to allocate memory, return */ @@ -507,14 +507,14 @@ void middle_pgsql_t::ways_set(osmid_t way_id, const idlist_t &nds, const taglist const char *tag_buf = pgsql_store_tags(tags,1); char *node_buf = pgsql_store_nodes(nds); int length = strlen(tag_buf) + strlen(node_buf) + 64; - buffer = (char *)alloca(length); + buffer = static_cast(alloca(length)); if (snprintf( buffer, length, "%" PRIdOSMID "\t%s\t%s\n", way_id, node_buf, tag_buf ) > (length-10)) { throw std::runtime_error((boost::format("Buffer overflow way id %1%") % way_id).str()); } pgsql_CopyData(__FUNCTION__, way_table->sql_conn, buffer); } else { - buffer = (char *)alloca(64); + buffer = static_cast(alloca(64)); char *ptr = buffer; paramValues[0] = ptr; sprintf(ptr, "%" PRIdOSMID, way_id); @@ -568,7 +568,7 @@ size_t middle_pgsql_t::ways_get_list(const idlist_t &ids, idlist_t &way_ids, char *tmp2; char const *paramValues[1]; - tmp2 = (char *)malloc(sizeof(char)*ids.size()*16); + tmp2 = static_cast(malloc(sizeof(char) * ids.size() * 16)); if (tmp2 == nullptr) return 0; //failed to allocate memory, return */ // create a list of ids in tmp2 to query the database */ @@ -721,7 +721,7 @@ void middle_pgsql_t::relations_set(osmid_t id, const memberlist_t &members, cons const char *member_buf = pgsql_store_tags(member_list,1); char *parts_buf = pgsql_store_nodes(all_parts); int length = strlen(member_buf) + strlen(tag_buf) + strlen(parts_buf) + 64; - buffer = (char *)alloca(length); + buffer = static_cast(alloca(length)); if (snprintf( buffer, length, "%" PRIdOSMID "\t%zu\t%zu\t%s\t%s\t%s\n", id, node_parts.size(), node_parts.size() + way_parts.size(), parts_buf, member_buf, tag_buf ) > (length-10)) { @@ -730,7 +730,7 @@ void middle_pgsql_t::relations_set(osmid_t id, const memberlist_t &members, cons free(tag_buf); pgsql_CopyData(__FUNCTION__, rel_table->sql_conn, buffer); } else { - buffer = (char *)alloca(64); + buffer = static_cast(alloca(64)); char *ptr = buffer; paramValues[0] = ptr; ptr += sprintf(ptr, "%" PRIdOSMID, id ) + 1; diff --git a/pgsql.cpp b/pgsql.cpp index 531fa429d..900fd7f51 100644 --- a/pgsql.cpp +++ b/pgsql.cpp @@ -53,7 +53,7 @@ int pgsql_exec(PGconn *sql_conn, const ExecStatusType expect, const char *fmt, . /* Based on vprintf manual page */ /* Guess we need no more than 100 bytes. */ - if ((sql = (char *)malloc(size)) == nullptr) + if ((sql = static_cast(malloc(size))) == nullptr) throw std::runtime_error("Memory allocation failed in pgsql_exec"); while (1) { @@ -69,7 +69,7 @@ int pgsql_exec(PGconn *sql_conn, const ExecStatusType expect, const char *fmt, . size = n+1; /* precisely what is needed */ else /* glibc 2.0 */ size *= 2; /* twice the old size */ - if ((nsql = (char *)realloc (sql, size)) == nullptr) { + if ((nsql = static_cast(realloc (sql, size))) == nullptr) { free(sql); throw std::runtime_error("Memory re-allocation failed in pgsql_exec"); } else { diff --git a/sprompt.cpp b/sprompt.cpp index 9a4771ba3..fece34ddc 100644 --- a/sprompt.cpp +++ b/sprompt.cpp @@ -87,7 +87,7 @@ simple_prompt(const char *prompt, int maxlen, int echo) #endif #endif - destination = (char *) malloc(maxlen + 1); + destination = static_cast(malloc(maxlen + 1)); if (!destination) return NULL; @@ -125,7 +125,7 @@ simple_prompt(const char *prompt, int maxlen, int echo) if (!echo) { /* get a new handle to turn echo off */ - t_orig = (LPDWORD) malloc(sizeof(DWORD)); + t_orig = static_cast(malloc(sizeof(DWORD))); t = GetStdHandle(STD_INPUT_HANDLE); /* save the old configuration first */ From c1a540819604cc53e623897b2ddaf5111d745b66 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Wed, 1 Jun 2016 18:37:36 +0200 Subject: [PATCH 3/5] replace malloc() with new While at it let std::unique_ptr handle the memory management. --- middle-pgsql.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/middle-pgsql.cpp b/middle-pgsql.cpp index b176ec4ff..f7c55cfa1 100644 --- a/middle-pgsql.cpp +++ b/middle-pgsql.cpp @@ -565,25 +565,24 @@ size_t middle_pgsql_t::ways_get_list(const idlist_t &ids, idlist_t &way_ids, return 0; char tmp[16]; - char *tmp2; + std::unique_ptr tmp2(new (std::nothrow) char[ids.size() * 16]); char const *paramValues[1]; - tmp2 = static_cast(malloc(sizeof(char) * ids.size() * 16)); if (tmp2 == nullptr) return 0; //failed to allocate memory, return */ // create a list of ids in tmp2 to query the database */ - sprintf(tmp2, "{"); + sprintf(tmp2.get(), "{"); for(idlist_t::const_iterator it = ids.begin(); it != ids.end(); ++it) { snprintf(tmp, sizeof(tmp), "%" PRIdOSMID ",", *it); - strncat(tmp2,tmp, sizeof(char)*(ids.size()*16 - 2)); + strncat(tmp2.get(), tmp, sizeof(char)*(ids.size()*16 - 2)); } - tmp2[strlen(tmp2) - 1] = '}'; // replace last , with } to complete list of ids*/ + tmp2[strlen(tmp2.get()) - 1] = '}'; // replace last , with } to complete list of ids*/ pgsql_endCopy(way_table); PGconn *sql_conn = way_table->sql_conn; - paramValues[0] = tmp2; + paramValues[0] = tmp2.get(); PGresult *res = pgsql_execPrepared(sql_conn, "get_way_list", 1, paramValues, PGRES_TUPLES_OK); int countPG = PQntuples(res); @@ -623,7 +622,6 @@ size_t middle_pgsql_t::ways_get_list(const idlist_t &ids, idlist_t &way_ids, assert(way_ids.size() <= ids.size()); PQclear(res); - free(tmp2); return way_ids.size(); } From 0beb1ebbf848a35831e85bfcf2c299c8f7eecd28 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Wed, 1 Jun 2016 18:41:48 +0200 Subject: [PATCH 4/5] entirely avoid dynamic allocation for a single DWORD, put it on the stack --- sprompt.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sprompt.cpp b/sprompt.cpp index fece34ddc..737b47bd2 100644 --- a/sprompt.cpp +++ b/sprompt.cpp @@ -83,7 +83,7 @@ simple_prompt(const char *prompt, int maxlen, int echo) #else #ifdef _WIN32 HANDLE t = NULL; - LPDWORD t_orig = NULL; + DWORD t_orig; #endif #endif @@ -125,11 +125,10 @@ simple_prompt(const char *prompt, int maxlen, int echo) if (!echo) { /* get a new handle to turn echo off */ - t_orig = static_cast(malloc(sizeof(DWORD))); t = GetStdHandle(STD_INPUT_HANDLE); /* save the old configuration first */ - GetConsoleMode(t, t_orig); + GetConsoleMode(t, &t_orig); /* set to the new mode */ SetConsoleMode(t, ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT); @@ -177,10 +176,9 @@ simple_prompt(const char *prompt, int maxlen, int echo) if (!echo) { /* reset to the original console mode */ - SetConsoleMode(t, *t_orig); + SetConsoleMode(t, t_orig); fputs("\n", termout); fflush(termout); - free(t_orig); } #endif #endif From 5bcca8f6469fdb6cb714661e08c31fbf7fb4b301 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer Date: Thu, 2 Jun 2016 17:37:10 +0200 Subject: [PATCH 5/5] avoid using alloca() to allocate a fixed size buffer Just directly declare a fixed size buffer of the given type. --- middle-pgsql.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/middle-pgsql.cpp b/middle-pgsql.cpp index f7c55cfa1..25f42fe04 100644 --- a/middle-pgsql.cpp +++ b/middle-pgsql.cpp @@ -313,7 +313,7 @@ void middle_pgsql_t::local_nodes_set(const osmid_t& id, const double& lat, } else { // Four params: id, lat, lon, tags */ const char *paramValues[4]; - char *buffer = static_cast(alloca(64)); + char buffer[64]; char *ptr = buffer; paramValues[0] = ptr; ptr += sprintf( ptr, "%" PRIdOSMID, id ) + 1; @@ -501,20 +501,19 @@ void middle_pgsql_t::ways_set(osmid_t way_id, const idlist_t &nds, const taglist { // Three params: id, nodes, tags */ const char *paramValues[4]; - char *buffer; if (way_table->copyMode) { const char *tag_buf = pgsql_store_tags(tags,1); char *node_buf = pgsql_store_nodes(nds); int length = strlen(tag_buf) + strlen(node_buf) + 64; - buffer = static_cast(alloca(length)); + char *buffer = static_cast(alloca(length)); if (snprintf( buffer, length, "%" PRIdOSMID "\t%s\t%s\n", way_id, node_buf, tag_buf ) > (length-10)) { throw std::runtime_error((boost::format("Buffer overflow way id %1%") % way_id).str()); } pgsql_CopyData(__FUNCTION__, way_table->sql_conn, buffer); } else { - buffer = static_cast(alloca(64)); + char buffer[64]; char *ptr = buffer; paramValues[0] = ptr; sprintf(ptr, "%" PRIdOSMID, way_id); @@ -684,7 +683,6 @@ void middle_pgsql_t::relations_set(osmid_t id, const memberlist_t &members, cons { // Params: id, way_off, rel_off, parts, members, tags */ const char *paramValues[6]; - char *buffer; taglist_t member_list; char buf[64]; @@ -719,7 +717,7 @@ void middle_pgsql_t::relations_set(osmid_t id, const memberlist_t &members, cons const char *member_buf = pgsql_store_tags(member_list,1); char *parts_buf = pgsql_store_nodes(all_parts); int length = strlen(member_buf) + strlen(tag_buf) + strlen(parts_buf) + 64; - buffer = static_cast(alloca(length)); + char *buffer = static_cast(alloca(length)); if (snprintf( buffer, length, "%" PRIdOSMID "\t%zu\t%zu\t%s\t%s\t%s\n", id, node_parts.size(), node_parts.size() + way_parts.size(), parts_buf, member_buf, tag_buf ) > (length-10)) { @@ -728,7 +726,7 @@ void middle_pgsql_t::relations_set(osmid_t id, const memberlist_t &members, cons free(tag_buf); pgsql_CopyData(__FUNCTION__, rel_table->sql_conn, buffer); } else { - buffer = static_cast(alloca(64)); + char buffer[64]; char *ptr = buffer; paramValues[0] = ptr; ptr += sprintf(ptr, "%" PRIdOSMID, id ) + 1;