From 783e8edf719be4e355db8eab5faaa13f676364e5 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Mon, 7 Apr 2014 15:07:07 -0400 Subject: [PATCH 01/27] dont reset coloring when it wasn't set in the first place --- Util/SimpleLogger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Util/SimpleLogger.h b/Util/SimpleLogger.h index bf531f0d2..a421b1e1c 100644 --- a/Util/SimpleLogger.h +++ b/Util/SimpleLogger.h @@ -104,7 +104,7 @@ public: if(!LogPolicy::GetInstance().IsMute()) { switch(level) { case logINFO: - std::cout << os.str() << COL_RESET << std::endl; + std::cout << os.str() << std::endl; break; case logWARNING: std::cerr << RED << os.str() << COL_RESET << std::endl; From b8f882dba47c0e22fb27f64b9e7d2b47f43beb68 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Mon, 7 Apr 2014 15:11:04 -0400 Subject: [PATCH 02/27] dont reset coloring when it wasn't set in the first place, partially fixes Windows woes. See #979 --- Util/SimpleLogger.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Util/SimpleLogger.h b/Util/SimpleLogger.h index a421b1e1c..144ca73b7 100644 --- a/Util/SimpleLogger.h +++ b/Util/SimpleLogger.h @@ -36,7 +36,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include enum LogLevel { logINFO, logWARNING, logDEBUG }; -static boost::mutex logger_mutex; +static boost::mutex logger_mutex; const char COL_RESET[] = "\x1b[0m"; const char RED[] = "\x1b[31m"; const char GREEN[] = "\x1b[32m"; @@ -79,7 +79,7 @@ public: boost::mutex::scoped_lock lock(logger_mutex); level = l; os << "["; - switch(level) { + switch(level) { case logINFO: os << "info"; break; From 6814926f05b27fde19a0bb11cd57e0d2b83852da Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 11:34:15 -0400 Subject: [PATCH 03/27] catch exceptions that may occur, coverity issue 1198846 --- Tools/unlock_all_mutexes.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Tools/unlock_all_mutexes.cpp b/Tools/unlock_all_mutexes.cpp index eece57ddc..bc9429e98 100644 --- a/Tools/unlock_all_mutexes.cpp +++ b/Tools/unlock_all_mutexes.cpp @@ -33,13 +33,20 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. int main() { LogPolicy::GetInstance().Unmute(); - SimpleLogger().Write() << - "starting up engines, " << g_GIT_DESCRIPTION << ", " << - "compiled at " << __DATE__ << ", " __TIME__; - SimpleLogger().Write() << "Releasing all locks"; - SharedBarriers barrier; - barrier.pending_update_mutex.unlock(); - barrier.query_mutex.unlock(); - barrier.update_mutex.unlock(); + try + { + SimpleLogger().Write() << + "starting up engines, " << g_GIT_DESCRIPTION << ", " << + "compiled at " << __DATE__ << ", " __TIME__; + SimpleLogger().Write() << "Releasing all locks"; + SharedBarriers barrier; + barrier.pending_update_mutex.unlock(); + barrier.query_mutex.unlock(); + barrier.update_mutex.unlock(); + } + catch(const std::exception & e) + { + SimpleLogger().Write(logWARNING) << "[excpetion] " << e.what(); + } return 0; } From f581396f1d1dc01b5d51fc576ee4c87c3bad3070 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 11:44:56 -0400 Subject: [PATCH 04/27] add clang format style file (to change) --- .clang-format | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 000000000..b7a8ad674 --- /dev/null +++ b/.clang-format @@ -0,0 +1,54 @@ +--- +Language: Cpp +# BasedOnStyle: LLVM +AccessModifierOffset: -2 +ConstructorInitializerIndentWidth: 4 +AlignEscapedNewlinesLeft: false +AlignTrailingComments: true +AllowAllParametersOfDeclarationOnNextLine: true +AllowShortIfStatementsOnASingleLine: false +AllowShortLoopsOnASingleLine: false +AllowShortFunctionsOnASingleLine: true +AlwaysBreakTemplateDeclarations: false +AlwaysBreakBeforeMultilineStrings: false +BreakBeforeBinaryOperators: false +BreakBeforeTernaryOperators: true +BreakConstructorInitializersBeforeComma: false +BinPackParameters: true +ColumnLimit: 80 +ConstructorInitializerAllOnOneLineOrOnePerLine: false +DerivePointerBinding: false +ExperimentalAutoDetectBinPacking: false +IndentCaseLabels: false +MaxEmptyLinesToKeep: 1 +KeepEmptyLinesAtTheStartOfBlocks: true +NamespaceIndentation: None +ObjCSpaceAfterProperty: false +ObjCSpaceBeforeProtocolList: true +PenaltyBreakBeforeFirstCallParameter: 19 +PenaltyBreakComment: 300 +PenaltyBreakString: 1000 +PenaltyBreakFirstLessLess: 120 +PenaltyExcessCharacter: 1000000 +PenaltyReturnTypeOnItsOwnLine: 60 +PointerBindsToType: false +SpacesBeforeTrailingComments: 1 +Cpp11BracedListStyle: true +Standard: Cpp11 +IndentWidth: 4 +TabWidth: 8 +UseTab: Never +BreakBeforeBraces: Allman +IndentFunctionDeclarationAfterType: false +SpacesInParentheses: false +SpacesInAngles: false +SpaceInEmptyParentheses: false +SpacesInCStyleCastParentheses: false +SpacesInContainerLiterals: true +SpaceBeforeAssignmentOperators: true +ContinuationIndentWidth: 4 +CommentPragmas: '^ IWYU pragma:' +ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ] +SpaceBeforeParens: ControlStatements +... + From a92c764945e11ccefb5d9f8c73a60e277f66906c Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 11:48:48 -0400 Subject: [PATCH 05/27] allow 120 char wide columns --- .clang-format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-format b/.clang-format index b7a8ad674..3efd035a1 100644 --- a/.clang-format +++ b/.clang-format @@ -15,7 +15,7 @@ BreakBeforeBinaryOperators: false BreakBeforeTernaryOperators: true BreakConstructorInitializersBeforeComma: false BinPackParameters: true -ColumnLimit: 80 +ColumnLimit: 120 ConstructorInitializerAllOnOneLineOrOnePerLine: false DerivePointerBinding: false ExperimentalAutoDetectBinPacking: false From b11e39554f4fd2ecac4cfc6ec23c11c20cf7607b Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 11:55:59 -0400 Subject: [PATCH 06/27] don't binpack parameters on 100 column width --- .clang-format | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.clang-format b/.clang-format index 3efd035a1..f5577f5de 100644 --- a/.clang-format +++ b/.clang-format @@ -14,8 +14,8 @@ AlwaysBreakBeforeMultilineStrings: false BreakBeforeBinaryOperators: false BreakBeforeTernaryOperators: true BreakConstructorInitializersBeforeComma: false -BinPackParameters: true -ColumnLimit: 120 +BinPackParameters: false +ColumnLimit: 100 ConstructorInitializerAllOnOneLineOrOnePerLine: false DerivePointerBinding: false ExperimentalAutoDetectBinPacking: false @@ -29,7 +29,7 @@ PenaltyBreakBeforeFirstCallParameter: 19 PenaltyBreakComment: 300 PenaltyBreakString: 1000 PenaltyBreakFirstLessLess: 120 -PenaltyExcessCharacter: 1000000 +PenaltyExcessCharacter: 1000 PenaltyReturnTypeOnItsOwnLine: 60 PointerBindsToType: false SpacesBeforeTrailingComments: 1 From a1b5429f4ebe23d156c581bd5f0f42750c7485b7 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 11:57:15 -0400 Subject: [PATCH 07/27] catch exceptions that may occur, coverity issue 1198845 --- Tools/componentAnalysis.cpp | 160 +++++++++++++++++------------------- 1 file changed, 76 insertions(+), 84 deletions(-) diff --git a/Tools/componentAnalysis.cpp b/Tools/componentAnalysis.cpp index 14a23956f..3f216b71b 100644 --- a/Tools/componentAnalysis.cpp +++ b/Tools/componentAnalysis.cpp @@ -46,99 +46,91 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include -typedef QueryEdge::EdgeData EdgeData; +typedef QueryEdge::EdgeData EdgeData; typedef DynamicGraph::InputEdge InputEdge; -std::vector internal_to_external_node_map; -std::vector restrictions_vector; -std::vector bollard_node_IDs_vector; -std::vector traffic_light_node_IDs_vector; +std::vector internal_to_external_node_map; +std::vector restrictions_vector; +std::vector bollard_node_IDs_vector; +std::vector traffic_light_node_IDs_vector; -int main (int argc, char * argv[]) { +int main(int argc, char *argv[]) +{ LogPolicy::GetInstance().Unmute(); - if(argc < 3) { - SimpleLogger().Write(logWARNING) << - "usage:\n" << argv[0] << " "; + if (argc < 3) + { + SimpleLogger().Write(logWARNING) << "usage:\n" << argv[0] << " "; return -1; } - SimpleLogger().Write() << - "Using restrictions from file: " << argv[2]; - std::ifstream restriction_ifstream(argv[2], std::ios::binary); - const UUID uuid_orig; - UUID uuid_loaded; - restriction_ifstream.read((char *) &uuid_loaded, sizeof(UUID)); + try + { + SimpleLogger().Write() << "Using restrictions from file: " << argv[2]; + std::ifstream restriction_ifstream(argv[2], std::ios::binary); + const UUID uuid_orig; + UUID uuid_loaded; + restriction_ifstream.read((char *)&uuid_loaded, sizeof(UUID)); - if( !uuid_loaded.TestGraphUtil(uuid_orig) ) { - SimpleLogger().Write(logWARNING) << - argv[2] << " was prepared with a different build. " - "Reprocess to get rid of this warning."; + if (!uuid_loaded.TestGraphUtil(uuid_orig)) + { + SimpleLogger().Write(logWARNING) << argv[2] << " was prepared with a different build. " + "Reprocess to get rid of this warning."; + } + + if (!restriction_ifstream.good()) + { + throw OSRMException("Could not access files"); + } + uint32_t usable_restriction_count = 0; + restriction_ifstream.read((char *)&usable_restriction_count, sizeof(uint32_t)); + restrictions_vector.resize(usable_restriction_count); + + restriction_ifstream.read((char *)&(restrictions_vector[0]), + usable_restriction_count * sizeof(TurnRestriction)); + restriction_ifstream.close(); + + std::ifstream input_stream; + input_stream.open(argv[1], std::ifstream::in | std::ifstream::binary); + + if (!input_stream.is_open()) + { + throw OSRMException("Cannot open osrm file"); + } + + std::vector edge_list; + NodeID node_based_node_count = readBinaryOSRMGraphFromStream( + input_stream, edge_list, bollard_node_IDs_vector, traffic_light_node_IDs_vector, + &internal_to_external_node_map, restrictions_vector); + input_stream.close(); + + BOOST_ASSERT_MSG(restrictions_vector.size() == usable_restriction_count, + "size of restrictions_vector changed"); + + SimpleLogger().Write() << restrictions_vector.size() << " restrictions, " + << bollard_node_IDs_vector.size() << " bollard nodes, " + << traffic_light_node_IDs_vector.size() << " traffic lights"; + + /*** + * Building an edge-expanded graph from node-based input an turn + * restrictions + */ + + SimpleLogger().Write() << "Starting SCC graph traversal"; + TarjanSCC *tarjan = new TarjanSCC(node_based_node_count, edge_list, bollard_node_IDs_vector, + traffic_light_node_IDs_vector, restrictions_vector, + internal_to_external_node_map); + std::vector().swap(edge_list); + + tarjan->Run(); + + std::vector().swap(restrictions_vector); + std::vector().swap(bollard_node_IDs_vector); + std::vector().swap(traffic_light_node_IDs_vector); + SimpleLogger().Write() << "finished component analysis"; } - - if(!restriction_ifstream.good()) { - throw OSRMException("Could not access files"); + catch (const std::exception &e) + { + SimpleLogger().Write(logWARNING) << "[exception] " << e.what(); } - uint32_t usable_restriction_count = 0; - restriction_ifstream.read( - (char*)&usable_restriction_count, - sizeof(uint32_t) - ); - restrictions_vector.resize(usable_restriction_count); - - restriction_ifstream.read( - (char *)&(restrictions_vector[0]), - usable_restriction_count*sizeof(TurnRestriction) - ); - restriction_ifstream.close(); - - std::ifstream input_stream; - input_stream.open( argv[1], std::ifstream::in | std::ifstream::binary ); - - if (!input_stream.is_open()) { - throw OSRMException("Cannot open osrm file"); - } - - std::vector edge_list; - NodeID node_based_node_count = readBinaryOSRMGraphFromStream( - input_stream, - edge_list, - bollard_node_IDs_vector, - traffic_light_node_IDs_vector, - &internal_to_external_node_map, - restrictions_vector - ); - input_stream.close(); - - BOOST_ASSERT_MSG( - restrictions_vector.size() == usable_restriction_count, - "size of restrictions_vector changed" - ); - - SimpleLogger().Write() << - restrictions_vector.size() << " restrictions, " << - bollard_node_IDs_vector.size() << " bollard nodes, " << - traffic_light_node_IDs_vector.size() << " traffic lights"; - - /*** - * Building an edge-expanded graph from node-based input an turn restrictions - */ - - SimpleLogger().Write() << "Starting SCC graph traversal"; - TarjanSCC * tarjan = new TarjanSCC ( - node_based_node_count, - edge_list, - bollard_node_IDs_vector, - traffic_light_node_IDs_vector, - restrictions_vector, - internal_to_external_node_map - ); - std::vector().swap(edge_list); - - tarjan->Run(); - - std::vector().swap(restrictions_vector); - std::vector().swap(bollard_node_IDs_vector); - std::vector().swap(traffic_light_node_IDs_vector); - SimpleLogger().Write() << "finished component analysis"; return 0; } From 3a1a51ac4676dde4170648e56778a5ec50a70279 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 11:59:48 -0400 Subject: [PATCH 08/27] fix resource leak, coverity issue 1198844 --- Algorithms/StronglyConnectedComponents.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Algorithms/StronglyConnectedComponents.h b/Algorithms/StronglyConnectedComponents.h index e0245ef38..be6ddb7e7 100644 --- a/Algorithms/StronglyConnectedComponents.h +++ b/Algorithms/StronglyConnectedComponents.h @@ -145,7 +145,7 @@ public: if(restriction_iterator == m_restriction_map.end()) { index = m_restriction_bucket_list.size(); m_restriction_bucket_list.resize(index+1); - m_restriction_map[restrictionSource] = index; + restriction_iterator->second = index; } else { index = restriction_iterator->second; //Map already contains an is_only_*-restriction From 9894f2e053375df47a8118ecf232cb715c99616b Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 12:03:19 -0400 Subject: [PATCH 09/27] check for invalid parameter, coverity issue 1198843, also reformat source --- Tools/io-benchmark.cpp | 305 +++++++++++++++++++---------------------- 1 file changed, 142 insertions(+), 163 deletions(-) diff --git a/Tools/io-benchmark.cpp b/Tools/io-benchmark.cpp index e235c1208..e1a215d96 100644 --- a/Tools/io-benchmark.cpp +++ b/Tools/io-benchmark.cpp @@ -48,45 +48,40 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. const unsigned number_of_elements = 268435456; -struct Statistics { double min, max, med, mean, dev; }; +struct Statistics +{ + double min, max, med, mean, dev; +}; -void RunStatistics(std::vector & timings_vector, Statistics & stats) { +void RunStatistics(std::vector &timings_vector, Statistics &stats) +{ std::sort(timings_vector.begin(), timings_vector.end()); stats.min = timings_vector.front(); stats.max = timings_vector.back(); - stats.med = timings_vector[timings_vector.size()/2]; - double primary_sum = std::accumulate( - timings_vector.begin(), - timings_vector.end(), - 0.0 - ); + stats.med = timings_vector[timings_vector.size() / 2]; + double primary_sum = std::accumulate(timings_vector.begin(), timings_vector.end(), 0.0); stats.mean = primary_sum / timings_vector.size(); - double primary_sq_sum = std::inner_product( timings_vector.begin(), - timings_vector.end(), - timings_vector.begin(), - 0.0 - ); - stats.dev = std::sqrt( - primary_sq_sum / timings_vector.size() - (stats.mean * stats.mean) - ); + double primary_sq_sum = std::inner_product( + timings_vector.begin(), timings_vector.end(), timings_vector.begin(), 0.0); + stats.dev = std::sqrt(primary_sq_sum / timings_vector.size() - (stats.mean * stats.mean)); } -int main (int argc, char * argv[]) { +int main(int argc, char *argv[]) +{ LogPolicy::GetInstance().Unmute(); - SimpleLogger().Write() << - "starting up engines, " << g_GIT_DESCRIPTION << ", " << - "compiled at " << __DATE__ << ", " __TIME__; + SimpleLogger().Write() << "starting up engines, " << g_GIT_DESCRIPTION << ", " + << "compiled at " << __DATE__ << ", " __TIME__; #ifdef __FreeBSD__ - SimpleLogger().Write() << "Not supported on FreeBSD"; - return 0; + SimpleLogger().Write() << "Not supported on FreeBSD"; + return 0; #endif - if( 1 == argc ) { - SimpleLogger().Write(logWARNING) << - "usage: " << argv[0] << " /path/on/device"; + if (1 == argc) + { + SimpleLogger().Write(logWARNING) << "usage: " << argv[0] << " /path/on/device"; return -1; } @@ -94,119 +89,105 @@ int main (int argc, char * argv[]) { test_path /= "osrm.tst"; SimpleLogger().Write(logDEBUG) << "temporary file: " << test_path.string(); - try { - //create files for testing - if( 2 == argc) { - //create file to test - if( boost::filesystem::exists(test_path) ) { + try + { + // create files for testing + if (2 == argc) + { + // create file to test + if (boost::filesystem::exists(test_path)) + { throw OSRMException("Data file already exists"); } double time1, time2; - int * random_array = new int[number_of_elements]; - std::generate ( - random_array, - random_array+number_of_elements, - std::rand - ); + int *random_array = new int[number_of_elements]; + std::generate(random_array, random_array + number_of_elements, std::rand); #ifdef __APPLE__ - FILE * fd = fopen(test_path.string().c_str(), "w"); + FILE *fd = fopen(test_path.string().c_str(), "w"); fcntl(fileno(fd), F_NOCACHE, 1); fcntl(fileno(fd), F_RDAHEAD, 0); time1 = get_timestamp(); - write( - fileno(fd), - (char*)random_array, - number_of_elements*sizeof(unsigned) - ); + write(fileno(fd), (char *)random_array, number_of_elements * sizeof(unsigned)); time2 = get_timestamp(); fclose(fd); #endif #ifdef __linux__ - int f = open( - test_path.string().c_str(), - O_CREAT|O_TRUNC|O_WRONLY|O_SYNC, - S_IRWXU - ); + int f = + open(test_path.string().c_str(), O_CREAT | O_TRUNC | O_WRONLY | O_SYNC, S_IRWXU); time1 = get_timestamp(); - int ret = write( - f, - random_array, - number_of_elements*sizeof(unsigned) - ); - if(-1 == ret) { + int ret = write(f, random_array, number_of_elements * sizeof(unsigned)); + if (-1 == ret) + { throw OSRMException("could not write random data file"); } time2 = get_timestamp(); close(f); #endif delete[] random_array; - SimpleLogger().Write(logDEBUG) << - "writing raw 1GB took " << (time2-time1)*1000 << "ms"; - SimpleLogger().Write() << "raw write performance: " << - std::setprecision(5) << std::fixed << - 1024*1024/((time2-time1)*1000) << "MB/sec"; + SimpleLogger().Write(logDEBUG) << "writing raw 1GB took " << (time2 - time1) * 1000 + << "ms"; + SimpleLogger().Write() << "raw write performance: " << std::setprecision(5) + << std::fixed << 1024 * 1024 / ((time2 - time1) * 1000) + << "MB/sec"; - SimpleLogger().Write(logDEBUG) << - "finished creation of random data. Flush disk cache now!"; - - } else { + SimpleLogger().Write(logDEBUG) + << "finished creation of random data. Flush disk cache now!"; + } + else + { // // Run Non-Cached I/O benchmarks // - if( !boost::filesystem::exists(test_path) ) { + if (!boost::filesystem::exists(test_path)) + { throw OSRMException("data file does not exist"); } double time1, time2; - //volatiles do not get optimized + // volatiles do not get optimized Statistics stats; #ifdef __APPLE__ volatile unsigned single_block[1024]; - char * raw_array = new char[number_of_elements*sizeof(unsigned)]; - FILE * fd = fopen(test_path.string().c_str(), "r"); + char *raw_array = new char[number_of_elements * sizeof(unsigned)]; + FILE *fd = fopen(test_path.string().c_str(), "r"); fcntl(fileno(fd), F_NOCACHE, 1); fcntl(fileno(fd), F_RDAHEAD, 0); #endif #ifdef __linux__ - char * single_block = (char*) memalign( - 512, - 1024*sizeof(unsigned) - ); + char *single_block = (char *)memalign(512, 1024 * sizeof(unsigned)); - int f = open(test_path.string().c_str(), O_RDONLY|O_DIRECT|O_SYNC); - SimpleLogger().Write(logDEBUG) << - "opened, error: " << strerror(errno); - char * raw_array = (char*) memalign( - 512, - number_of_elements*sizeof(unsigned) - ); + int f = open(test_path.string().c_str(), O_RDONLY | O_DIRECT | O_SYNC); + if (-1 == f) + { + SimpleLogger().Write(logDEBUG) << "opened, error: " << strerror(errno); + return -1; + } + char *raw_array = (char *)memalign(512, number_of_elements * sizeof(unsigned)); #endif time1 = get_timestamp(); #ifdef __APPLE__ - read(fileno(fd), raw_array, number_of_elements*sizeof(unsigned)); + read(fileno(fd), raw_array, number_of_elements * sizeof(unsigned)); close(fileno(fd)); fd = fopen(test_path.string().c_str(), "r"); #endif #ifdef __linux__ - int ret = read(f, raw_array, number_of_elements*sizeof(unsigned)); - SimpleLogger().Write(logDEBUG) << - "read " << ret << " bytes, error: " << strerror(errno); + int ret = read(f, raw_array, number_of_elements * sizeof(unsigned)); + SimpleLogger().Write(logDEBUG) << "read " << ret + << " bytes, error: " << strerror(errno); close(f); - f = open(test_path.string().c_str(), O_RDONLY|O_DIRECT|O_SYNC); - SimpleLogger().Write(logDEBUG) << - "opened, error: " << strerror(errno); + f = open(test_path.string().c_str(), O_RDONLY | O_DIRECT | O_SYNC); + SimpleLogger().Write(logDEBUG) << "opened, error: " << strerror(errno); #endif time2 = get_timestamp(); - SimpleLogger().Write(logDEBUG) << - "reading raw 1GB took " << (time2-time1)*1000 << "ms"; - SimpleLogger().Write() << "raw read performance: " << - std::setprecision(5) << std::fixed << - 1024*1024/((time2-time1)*1000) << "MB/sec"; + SimpleLogger().Write(logDEBUG) << "reading raw 1GB took " << (time2 - time1) * 1000 + << "ms"; + SimpleLogger().Write() << "raw read performance: " << std::setprecision(5) << std::fixed + << 1024 * 1024 / ((time2 - time1) * 1000) << "MB/sec"; std::vector timing_results_raw_random; SimpleLogger().Write(logDEBUG) << "running 1000 random I/Os of 4KB"; @@ -217,60 +198,59 @@ int main (int argc, char * argv[]) { #ifdef __linux__ lseek(f, 0, SEEK_SET); #endif - //make 1000 random access, time each I/O seperately - unsigned number_of_blocks = (number_of_elements*sizeof(unsigned)-1)/4096; - for(unsigned i = 0; i < 1000; ++i) { - unsigned block_to_read = std::rand()%number_of_blocks; - off_t current_offset = block_to_read*4096; + // make 1000 random access, time each I/O seperately + unsigned number_of_blocks = (number_of_elements * sizeof(unsigned) - 1) / 4096; + for (unsigned i = 0; i < 1000; ++i) + { + unsigned block_to_read = std::rand() % number_of_blocks; + off_t current_offset = block_to_read * 4096; time1 = get_timestamp(); #ifdef __APPLE__ int ret1 = fseek(fd, current_offset, SEEK_SET); - int ret2 = read(fileno(fd), (char*)&single_block[0], 4096); + int ret2 = read(fileno(fd), (char *)&single_block[0], 4096); #endif #ifdef __FreeBSD__ - int ret1 = 0; - int ret2 = 0; + int ret1 = 0; + int ret2 = 0; #endif #ifdef __linux__ int ret1 = lseek(f, current_offset, SEEK_SET); - int ret2 = read(f, (char*)single_block, 4096); + int ret2 = read(f, (char *)single_block, 4096); #endif time2 = get_timestamp(); - if( ((off_t)-1) == ret1) { - SimpleLogger().Write(logWARNING) - << "offset: " << current_offset; - SimpleLogger().Write(logWARNING) - << "seek error " << strerror(errno); + if (((off_t) - 1) == ret1) + { + SimpleLogger().Write(logWARNING) << "offset: " << current_offset; + SimpleLogger().Write(logWARNING) << "seek error " << strerror(errno); throw OSRMException("seek error"); } - if(-1 == ret2) { - SimpleLogger().Write(logWARNING) - << "offset: " << current_offset; - SimpleLogger().Write(logWARNING) - << "read error " << strerror(errno); + if (-1 == ret2) + { + SimpleLogger().Write(logWARNING) << "offset: " << current_offset; + SimpleLogger().Write(logWARNING) << "read error " << strerror(errno); throw OSRMException("read error"); } - timing_results_raw_random.push_back((time2-time1)*1000.); + timing_results_raw_random.push_back((time2 - time1) * 1000.); } // Do statistics SimpleLogger().Write(logDEBUG) << "running raw random I/O statistics"; std::ofstream random_csv("random.csv", std::ios::trunc); - for(unsigned i = 0; i < timing_results_raw_random.size(); ++i) { + for (unsigned i = 0; i < timing_results_raw_random.size(); ++i) + { random_csv << i << ", " << timing_results_raw_random[i] << std::endl; } random_csv.close(); RunStatistics(timing_results_raw_random, stats); - SimpleLogger().Write() << "raw random I/O: " << - std::setprecision(5) << std::fixed << - "min: " << stats.min << "ms, " << - "mean: " << stats.mean << "ms, " << - "med: " << stats.med << "ms, " << - "max: " << stats.max << "ms, " << - "dev: " << stats.dev << "ms"; + SimpleLogger().Write() << "raw random I/O: " << std::setprecision(5) << std::fixed + << "min: " << stats.min << "ms, " + << "mean: " << stats.mean << "ms, " + << "med: " << stats.med << "ms, " + << "max: " << stats.max << "ms, " + << "dev: " << stats.dev << "ms"; std::vector timing_results_raw_seq; #ifdef __APPLE__ @@ -280,81 +260,80 @@ int main (int argc, char * argv[]) { lseek(f, 0, SEEK_SET); #endif - //read every 100th block - for( - unsigned i = 0; - i < 1000; - ++i - ) { - off_t current_offset = i*4096; + // read every 100th block + for (unsigned i = 0; i < 1000; ++i) + { + off_t current_offset = i * 4096; time1 = get_timestamp(); - #ifdef __APPLE__ +#ifdef __APPLE__ int ret1 = fseek(fd, current_offset, SEEK_SET); - int ret2 = read(fileno(fd), (char*)&single_block, 4096); - #endif + int ret2 = read(fileno(fd), (char *)&single_block, 4096); +#endif - #ifdef __FreeBSD__ - int ret1 = 0; - int ret2 = 0; - #endif +#ifdef __FreeBSD__ + int ret1 = 0; + int ret2 = 0; +#endif - #ifdef __linux__ +#ifdef __linux__ int ret1 = lseek(f, current_offset, SEEK_SET); - int ret2 = read(f, (char*)single_block, 4096); - #endif + int ret2 = read(f, (char *)single_block, 4096); +#endif time2 = get_timestamp(); - if( ((off_t)-1) == ret1) { - SimpleLogger().Write(logWARNING) - << "offset: " << current_offset; - SimpleLogger().Write(logWARNING) - << "seek error " << strerror(errno); + if (((off_t) - 1) == ret1) + { + SimpleLogger().Write(logWARNING) << "offset: " << current_offset; + SimpleLogger().Write(logWARNING) << "seek error " << strerror(errno); throw OSRMException("seek error"); } - if(-1 == ret2) { - SimpleLogger().Write(logWARNING) - << "offset: " << current_offset; - SimpleLogger().Write(logWARNING) - << "read error " << strerror(errno); + if (-1 == ret2) + { + SimpleLogger().Write(logWARNING) << "offset: " << current_offset; + SimpleLogger().Write(logWARNING) << "read error " << strerror(errno); throw OSRMException("read error"); } - timing_results_raw_seq.push_back((time2-time1)*1000.); + timing_results_raw_seq.push_back((time2 - time1) * 1000.); } - #ifdef __APPLE__ +#ifdef __APPLE__ fclose(fd); // free(single_element); free(raw_array); - // free(single_block); - #endif - #ifdef __linux__ +// free(single_block); +#endif +#ifdef __linux__ close(f); - #endif - //Do statistics +#endif + // Do statistics SimpleLogger().Write(logDEBUG) << "running sequential I/O statistics"; - //print simple statistics: min, max, median, variance + // print simple statistics: min, max, median, variance std::ofstream seq_csv("sequential.csv", std::ios::trunc); - for(unsigned i = 0; i < timing_results_raw_seq.size(); ++i) { + for (unsigned i = 0; i < timing_results_raw_seq.size(); ++i) + { seq_csv << i << ", " << timing_results_raw_seq[i] << std::endl; } seq_csv.close(); RunStatistics(timing_results_raw_seq, stats); - SimpleLogger().Write() << "raw sequential I/O: " << - std::setprecision(5) << std::fixed << - "min: " << stats.min << "ms, " << - "mean: " << stats.mean << "ms, " << - "med: " << stats.med << "ms, " << - "max: " << stats.max << "ms, " << - "dev: " << stats.dev << "ms"; + SimpleLogger().Write() << "raw sequential I/O: " << std::setprecision(5) << std::fixed + << "min: " << stats.min << "ms, " + << "mean: " << stats.mean << "ms, " + << "med: " << stats.med << "ms, " + << "max: " << stats.max << "ms, " + << "dev: " << stats.dev << "ms"; - if( boost::filesystem::exists(test_path) ) { + if (boost::filesystem::exists(test_path)) + { boost::filesystem::remove(test_path); SimpleLogger().Write(logDEBUG) << "removing temporary files"; } } - } catch ( const std::exception & e ) { + } + catch (const std::exception &e) + { SimpleLogger().Write(logWARNING) << "caught exception: " << e.what(); SimpleLogger().Write(logWARNING) << "cleaning up, and exiting"; - if(boost::filesystem::exists(test_path)) { + if (boost::filesystem::exists(test_path)) + { boost::filesystem::remove(test_path); SimpleLogger().Write(logWARNING) << "removing temporary files"; } From 67efd150d4be9d66c320f51be91735187dc62e0a Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 18:12:45 -0400 Subject: [PATCH 10/27] make all error messages JSON --- Include/osrm/Reply.h | 4 ++-- Server/RequestHandler.cpp | 16 +++++----------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/Include/osrm/Reply.h b/Include/osrm/Reply.h index 8e9b31750..82f2d2f41 100644 --- a/Include/osrm/Reply.h +++ b/Include/osrm/Reply.h @@ -37,8 +37,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace http { const char okHTML[] = ""; -const char badRequestHTML[] = "Bad Request

400 Bad Request

"; -const char internalServerErrorHTML[] = "Internal Server Error

500 Internal Server Error

"; +const char badRequestHTML[] = "{\"status\": 400,\"status_message\":\"Bad Request\"}"; +const char internalServerErrorHTML[] = "{\"status\": 500,\"status_message\":\"Internal Server Error\"}"; const char seperators[] = { ':', ' ' }; const char crlf[] = { '\r', '\n' }; const std::string okString = "HTTP/1.0 200 OK\r\n"; diff --git a/Server/RequestHandler.cpp b/Server/RequestHandler.cpp index ba6d20629..dc3b88aad 100644 --- a/Server/RequestHandler.cpp +++ b/Server/RequestHandler.cpp @@ -76,21 +76,15 @@ void RequestHandler::handle_request(const http::Request& req, http::Reply& rep){ if ( !result || (it != request.end()) ) { rep = http::Reply::StockReply(http::Reply::badRequest); + rep.content.clear(); const int position = std::distance(request.begin(), it); + rep.content.push_back( + "{\"status\":400,\"status_message\":\"Query string malformed close to position " + ); std::string tmp_position_string; intToString(position, tmp_position_string); - rep.content.push_back( - "Input seems to be malformed close to position " - "
"
-            );
-            rep.content.push_back( request );
             rep.content.push_back(tmp_position_string);
-            rep.content.push_back("
");
-            const unsigned end = std::distance(request.begin(), it);
-            for(unsigned i = 0; i < end; ++i) {
-                rep.content.push_back(" ");
-            }
-            rep.content.push_back("^
"); + rep.content.push_back("\"}"); } else { //parsing done, lets call the right plugin to handle the request BOOST_ASSERT_MSG( From 1222aa6d25931f010c62eed83d1b2d9918213cac Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 9 Apr 2014 23:10:58 -0400 Subject: [PATCH 11/27] fix inverted logic --- datastore.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/datastore.cpp b/datastore.cpp index 8eefc0fba..8d3dccd8d 100644 --- a/datastore.cpp +++ b/datastore.cpp @@ -200,13 +200,15 @@ int main( const int argc, const char * argv[] ) { UUID uuid_loaded, uuid_orig; hsgr_input_stream.read((char *)&uuid_loaded, sizeof(UUID)); - if( !uuid_loaded.TestGraphUtil(uuid_orig) ) { - SimpleLogger().Write(logWARNING) << - ".hsgr was prepared with different build. " - "Reprocess to get rid of this warning."; - } else { + if (uuid_loaded.TestGraphUtil(uuid_orig)) + { SimpleLogger().Write(logDEBUG) << "UUID checked out ok"; } + else + { + SimpleLogger().Write(logWARNING) << ".hsgr was prepared with different build. " + "Reprocess to get rid of this warning."; + } // load checksum unsigned checksum = 0; From 3368b492b9800cbf22baff27cb5f285c2529729c Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 16:13:48 +0200 Subject: [PATCH 12/27] make single paramter c'tors explicit (thx flint) --- Util/IniFile.h | 2 +- Util/OSRMException.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Util/IniFile.h b/Util/IniFile.h index 0a9c26216..9ca85a3fe 100644 --- a/Util/IniFile.h +++ b/Util/IniFile.h @@ -35,7 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. class IniFile { public: - IniFile(const char * config_filename); + explicit IniFile(const char * config_filename); std::string GetParameter(const std::string & key); diff --git a/Util/OSRMException.h b/Util/OSRMException.h index 322124435..32597d357 100644 --- a/Util/OSRMException.h +++ b/Util/OSRMException.h @@ -33,8 +33,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. class OSRMException: public std::exception { public: - OSRMException(const char * message) : message(message) {} - OSRMException(const std::string & message) : message(message) {} + explicit OSRMException(const char * message) : message(message) {} + explicit OSRMException(const std::string & message) : message(message) {} virtual ~OSRMException() throw() {} private: virtual const char* what() const throw() { From 6d52a7d3d414fcf59c986543a4886c2c5f909a8d Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 16:15:49 +0200 Subject: [PATCH 13/27] make single paramter c'tors explicit (thx flint) --- Library/OSRM.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/OSRM.h b/Library/OSRM.h index 738c977a0..be137b8c5 100644 --- a/Library/OSRM.h +++ b/Library/OSRM.h @@ -40,7 +40,7 @@ class OSRM { private: OSRM_impl * OSRM_pimpl_; public: - OSRM( + explicit OSRM( const ServerPaths & paths, const bool use_shared_memory = false ); From e7db076648cd3e713be5bf1c865fe5a81ee2c34b Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 16:16:42 +0200 Subject: [PATCH 14/27] fix include order (thx flint) --- Library/OSRM_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/OSRM_impl.cpp b/Library/OSRM_impl.cpp index 51846ab26..02acbc98a 100644 --- a/Library/OSRM_impl.cpp +++ b/Library/OSRM_impl.cpp @@ -25,8 +25,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "OSRM.h" #include "OSRM_impl.h" +#include "OSRM.h" #include "../Plugins/HelloWorldPlugin.h" #include "../Plugins/LocatePlugin.h" From f90993be86dc8e8636ece757de17d772ad7e0f87 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 16:21:16 +0200 Subject: [PATCH 15/27] make single argument c'tor explicit (thx flint) --- RoutingAlgorithms/BasicRoutingInterface.h | 2 +- Server/APIGrammar.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/RoutingAlgorithms/BasicRoutingInterface.h b/RoutingAlgorithms/BasicRoutingInterface.h index 1335f43af..00bb2e167 100644 --- a/RoutingAlgorithms/BasicRoutingInterface.h +++ b/RoutingAlgorithms/BasicRoutingInterface.h @@ -54,7 +54,7 @@ private: protected: DataFacadeT * facade; public: - BasicRoutingInterface( DataFacadeT * facade ) : facade(facade) { } + explicit BasicRoutingInterface( DataFacadeT * facade ) : facade(facade) { } virtual ~BasicRoutingInterface(){ }; inline void RoutingStep( diff --git a/Server/APIGrammar.h b/Server/APIGrammar.h index f377e6f10..4e9a21e8a 100644 --- a/Server/APIGrammar.h +++ b/Server/APIGrammar.h @@ -36,7 +36,7 @@ namespace qi = boost::spirit::qi; template struct APIGrammar : qi::grammar { - APIGrammar(HandlerT * h) : APIGrammar::base_type(api_call), handler(h) { + explicit APIGrammar(HandlerT * h) : APIGrammar::base_type(api_call), handler(h) { api_call = qi::lit('/') >> string[boost::bind(&HandlerT::setService, handler, ::_1)] >> *(query); query = ('?') >> (+(zoom | output | jsonp | checksum | location | hint | cmp | language | instruction | geometry | alt_route | old_API) ) ; From c6902528b3844ccdbcac5aace4743f6fc8e7381b Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 16:22:13 +0200 Subject: [PATCH 16/27] reverse include order --- Server/RequestHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Server/RequestHandler.cpp b/Server/RequestHandler.cpp index dc3b88aad..9dceb6852 100644 --- a/Server/RequestHandler.cpp +++ b/Server/RequestHandler.cpp @@ -25,8 +25,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "APIGrammar.h" #include "RequestHandler.h" +#include "APIGrammar.h" #include "../Library/OSRM.h" #include "../Util/SimpleLogger.h" From 5a0d693c939fad6655b9b1e5a5d3129d28ff723b Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 16:59:40 +0200 Subject: [PATCH 17/27] several lints fixed that were detected by facebook's flint --- Contractor/EdgeBasedGraphFactory.cpp | 4 ++-- DataStructures/BinaryHeap.h | 8 ++++---- DataStructures/ConcurrentQueue.h | 2 +- DataStructures/DeallocatingVector.h | 4 ++-- DataStructures/DynamicGraph.h | 2 +- DataStructures/HashTable.h | 2 +- DataStructures/ImportEdge.h | 2 +- DataStructures/LRUCache.h | 2 +- DataStructures/Percent.h | 2 +- DataStructures/QueryNode.h | 6 +++--- DataStructures/Restriction.h | 7 +++---- DataStructures/SearchEngine.h | 2 +- DataStructures/SearchEngineData.h | 2 +- DataStructures/SharedMemoryVectorWrapper.h | 2 +- DataStructures/StaticKDTree.h | 4 ++-- DataStructures/XORFastHashStorage.h | 2 +- Extractor/ExtractorCallbacks.cpp | 3 ++- Extractor/ScriptingEnvironment.h | 2 +- Plugins/BasePlugin.h | 4 ++++ Plugins/NearestPlugin.h | 2 +- Plugins/TimestampPlugin.h | 2 +- Plugins/ViaRoutePlugin.h | 2 +- Server/DataStructures/InternalDataFacade.h | 2 +- Server/Server.h | 3 ++- 24 files changed, 39 insertions(+), 34 deletions(-) diff --git a/Contractor/EdgeBasedGraphFactory.cpp b/Contractor/EdgeBasedGraphFactory.cpp index e765c90b4..fef577840 100644 --- a/Contractor/EdgeBasedGraphFactory.cpp +++ b/Contractor/EdgeBasedGraphFactory.cpp @@ -47,11 +47,11 @@ EdgeBasedGraphFactory::EdgeBasedGraphFactory( std::vector & barrier_node_list, std::vector & traffic_light_node_list, std::vector & input_restrictions_list, - std::vector & m_node_info_list, + std::vector & node_info_list, SpeedProfileProperties speed_profile ) : speed_profile(speed_profile), m_turn_restrictions_count(0), - m_node_info_list(m_node_info_list) + m_node_info_list(node_info_list) { BOOST_FOREACH(const TurnRestriction & restriction, input_restrictions_list) { std::pair restriction_source = diff --git a/DataStructures/BinaryHeap.h b/DataStructures/BinaryHeap.h index 7cd17f66f..9edd600e3 100644 --- a/DataStructures/BinaryHeap.h +++ b/DataStructures/BinaryHeap.h @@ -43,7 +43,7 @@ template< typename NodeID, typename Key > class ArrayStorage { public: - ArrayStorage( size_t size ) : positions( new Key[size] ) { + explicit ArrayStorage( size_t size ) : positions( new Key[size] ) { memset(positions, 0, size*sizeof(Key)); } @@ -65,7 +65,7 @@ template< typename NodeID, typename Key > class MapStorage { public: - MapStorage( size_t ) {} + explicit MapStorage( size_t ) {} Key &operator[]( NodeID node ) { return nodes[node]; @@ -87,7 +87,7 @@ class UnorderedMapStorage { typedef typename UnorderedMapType::const_iterator UnorderedMapConstIterator; public: - UnorderedMapStorage( size_t ) { + explicit UnorderedMapStorage( size_t ) { //hash table gets 1000 Buckets nodes.rehash(1000); } @@ -119,7 +119,7 @@ public: typedef Weight WeightType; typedef Data DataType; - BinaryHeap( size_t maxID ) + explicit BinaryHeap( size_t maxID ) : nodeIndex( maxID ) { diff --git a/DataStructures/ConcurrentQueue.h b/DataStructures/ConcurrentQueue.h index ea52ef864..bc610583e 100644 --- a/DataStructures/ConcurrentQueue.h +++ b/DataStructures/ConcurrentQueue.h @@ -40,7 +40,7 @@ template class ConcurrentQueue { public: - ConcurrentQueue(const size_t max_size) : m_internal_queue(max_size) { } + explicit ConcurrentQueue(const size_t max_size) : m_internal_queue(max_size) { } inline void push(const Data & data) { boost::mutex::scoped_lock lock(m_mutex); diff --git a/DataStructures/DeallocatingVector.h b/DataStructures/DeallocatingVector.h index eef6d338d..4db1ec756 100644 --- a/DataStructures/DeallocatingVector.h +++ b/DataStructures/DeallocatingVector.h @@ -96,10 +96,10 @@ public: DeallocatingVectorIterator() {} template - DeallocatingVectorIterator(const DeallocatingVectorIterator & r) : mState(r.mState) {} + explicit DeallocatingVectorIterator(const DeallocatingVectorIterator & r) : mState(r.mState) {} DeallocatingVectorIterator(std::size_t idx, std::vector & input_list) : mState(idx, input_list) {} - DeallocatingVectorIterator(const DeallocatingVectorIteratorState & r) : mState(r) {} + explicit DeallocatingVectorIterator(const DeallocatingVectorIteratorState & r) : mState(r) {} template DeallocatingVectorIterator& operator=(const DeallocatingVectorIterator &r) { diff --git a/DataStructures/DynamicGraph.h b/DataStructures/DynamicGraph.h index 1ad5cc9a2..4a1b796ed 100644 --- a/DataStructures/DynamicGraph.h +++ b/DataStructures/DynamicGraph.h @@ -57,7 +57,7 @@ class DynamicGraph { }; //Constructs an empty graph with a given number of nodes. - DynamicGraph( int32_t nodes ) : m_numNodes(nodes), m_numEdges(0) { + explicit DynamicGraph( int32_t nodes ) : m_numNodes(nodes), m_numEdges(0) { m_nodes.reserve( m_numNodes ); m_nodes.resize( m_numNodes ); diff --git a/DataStructures/HashTable.h b/DataStructures/HashTable.h index 1e9c3333c..b4984e4b5 100644 --- a/DataStructures/HashTable.h +++ b/DataStructures/HashTable.h @@ -40,7 +40,7 @@ public: HashTable() : super() { } - HashTable(const unsigned size) : super(size) { } + explicit HashTable(const unsigned size) : super(size) { } inline void Add( KeyT const & key, ValueT const & value) { super::emplace(std::make_pair(key, value)); diff --git a/DataStructures/ImportEdge.h b/DataStructures/ImportEdge.h index 23e30e2fb..33e52cc12 100644 --- a/DataStructures/ImportEdge.h +++ b/DataStructures/ImportEdge.h @@ -129,7 +129,7 @@ public: } template - EdgeBasedEdge(const EdgeT & myEdge ) : + explicit EdgeBasedEdge(const EdgeT & myEdge ) : m_source(myEdge.source), m_target(myEdge.target), m_edgeID(myEdge.data.via), diff --git a/DataStructures/LRUCache.h b/DataStructures/LRUCache.h index 8e7733171..1dbe31709 100644 --- a/DataStructures/LRUCache.h +++ b/DataStructures/LRUCache.h @@ -43,7 +43,7 @@ private: std::list itemsInCache; boost::unordered_map::iterator > positionMap; public: - LRUCache(unsigned c) : capacity(c) {} + explicit LRUCache(unsigned c) : capacity(c) {} bool Holds(KeyT key) { if(positionMap.find(key) != positionMap.end()) { diff --git a/DataStructures/Percent.h b/DataStructures/Percent.h index dc2e7d0f4..13bf728d7 100644 --- a/DataStructures/Percent.h +++ b/DataStructures/Percent.h @@ -38,7 +38,7 @@ public: * @param maxValue the value that corresponds to 100% * @param step the progress is shown in steps of 'step' percent */ - Percent(unsigned maxValue, unsigned step = 5) { + explicit Percent(unsigned maxValue, unsigned step = 5) { reinit(maxValue, step); } diff --git a/DataStructures/QueryNode.h b/DataStructures/QueryNode.h index 89dccb47a..ae0dc4bac 100644 --- a/DataStructures/QueryNode.h +++ b/DataStructures/QueryNode.h @@ -25,8 +25,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#ifndef _NODE_COORDS_H -#define _NODE_COORDS_H +#ifndef QUERY_NODE_H +#define QUERY_NODE_H #include "../typedefs.h" @@ -83,4 +83,4 @@ struct NodeInfo { } }; -#endif //_NODE_COORDS_H +#endif //QUERY_NODE_H diff --git a/DataStructures/Restriction.h b/DataStructures/Restriction.h index 9f53ce3bf..429cc8acd 100644 --- a/DataStructures/Restriction.h +++ b/DataStructures/Restriction.h @@ -58,14 +58,13 @@ struct TurnRestriction { bool unused7:1; } flags; - TurnRestriction(NodeID viaNode) : + explicit TurnRestriction(NodeID viaNode) : viaNode(viaNode), fromNode(UINT_MAX), toNode(UINT_MAX) { - } - TurnRestriction(const bool isOnly = false) : + explicit TurnRestriction(const bool isOnly = false) : viaNode(UINT_MAX), fromNode(UINT_MAX), toNode(UINT_MAX) { @@ -91,7 +90,7 @@ struct InputRestrictionContainer { { restriction.viaNode = vn; } - InputRestrictionContainer( + explicit InputRestrictionContainer( bool isOnly = false ) : fromWay(UINT_MAX), diff --git a/DataStructures/SearchEngine.h b/DataStructures/SearchEngine.h index 025805cc4..f03bf93d9 100644 --- a/DataStructures/SearchEngine.h +++ b/DataStructures/SearchEngine.h @@ -54,7 +54,7 @@ public: ShortestPathRouting shortest_path; AlternativeRouting alternative_path; - SearchEngine( DataFacadeT * facade ) + explicit SearchEngine( DataFacadeT * facade ) : facade (facade), shortest_path (facade, engine_working_data), diff --git a/DataStructures/SearchEngineData.h b/DataStructures/SearchEngineData.h index 5d3e68046..dd7ac5f50 100644 --- a/DataStructures/SearchEngineData.h +++ b/DataStructures/SearchEngineData.h @@ -41,7 +41,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. struct _HeapData { NodeID parent; - _HeapData( NodeID p ) : parent(p) { } + /* explicit */ _HeapData( NodeID p ) : parent(p) { } }; // typedef StaticGraph QueryGraph; diff --git a/DataStructures/SharedMemoryVectorWrapper.h b/DataStructures/SharedMemoryVectorWrapper.h index 4524fa6c0..8ceff2489 100644 --- a/DataStructures/SharedMemoryVectorWrapper.h +++ b/DataStructures/SharedMemoryVectorWrapper.h @@ -41,7 +41,7 @@ template class ShMemIterator : public std::iterator { DataT * p; public: - ShMemIterator(DataT * x) : p(x) {} + explicit ShMemIterator(DataT * x) : p(x) {} ShMemIterator(const ShMemIterator & mit) : p(mit.p) {} ShMemIterator& operator++() { ++p; diff --git a/DataStructures/StaticKDTree.h b/DataStructures/StaticKDTree.h index 8a9e88876..200fc5cd5 100644 --- a/DataStructures/StaticKDTree.h +++ b/DataStructures/StaticKDTree.h @@ -99,7 +99,7 @@ public: } }; - StaticKDTree( std::vector< InputPoint > * points ){ + explicit StaticKDTree( std::vector< InputPoint > * points ){ BOOST_ASSERT( k > 0 ); BOOST_ASSERT ( points->size() > 0 ); size = points->size(); @@ -207,7 +207,7 @@ private: }; class Less { public: - Less( unsigned d ) { + explicit Less( unsigned d ) { dimension = d; BOOST_ASSERT( dimension < k ); } diff --git a/DataStructures/XORFastHashStorage.h b/DataStructures/XORFastHashStorage.h index e4e930fd9..ae5bf1f01 100644 --- a/DataStructures/XORFastHashStorage.h +++ b/DataStructures/XORFastHashStorage.h @@ -54,7 +54,7 @@ public: } }; - XORFastHashStorage( size_t ) : positions(2<<16), currentTimestamp(0) { } + explicit XORFastHashStorage( size_t ) : positions(2<<16), currentTimestamp(0) { } inline HashCell& operator[]( const NodeID node ) { unsigned short position = fastHash(node); diff --git a/Extractor/ExtractorCallbacks.cpp b/Extractor/ExtractorCallbacks.cpp index c1ef1326c..e97275749 100644 --- a/Extractor/ExtractorCallbacks.cpp +++ b/Extractor/ExtractorCallbacks.cpp @@ -25,10 +25,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include "ExtractorCallbacks.h" + #include "ExtractionContainers.h" #include "ExtractionHelperFunctions.h" #include "ExtractionWay.h" -#include "ExtractorCallbacks.h" #include "../DataStructures/Restriction.h" #include "../Util/SimpleLogger.h" diff --git a/Extractor/ScriptingEnvironment.h b/Extractor/ScriptingEnvironment.h index de3e96e8d..579df1fea 100644 --- a/Extractor/ScriptingEnvironment.h +++ b/Extractor/ScriptingEnvironment.h @@ -35,7 +35,7 @@ struct lua_State; class ScriptingEnvironment { public: ScriptingEnvironment(); - ScriptingEnvironment(const char * fileName); + explicit ScriptingEnvironment(const char * fileName); virtual ~ScriptingEnvironment(); lua_State * getLuaStateForThreadID(const int); diff --git a/Plugins/BasePlugin.h b/Plugins/BasePlugin.h index bf1ecf90f..d60ce7534 100644 --- a/Plugins/BasePlugin.h +++ b/Plugins/BasePlugin.h @@ -28,10 +28,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef BASEPLUGIN_H_ #define BASEPLUGIN_H_ +#include "../Util/StringUtil.h" + #include #include #include +#include + #include #include diff --git a/Plugins/NearestPlugin.h b/Plugins/NearestPlugin.h index 5a4bcbab7..0543dffe0 100644 --- a/Plugins/NearestPlugin.h +++ b/Plugins/NearestPlugin.h @@ -41,7 +41,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. template class NearestPlugin : public BasePlugin { public: - NearestPlugin(DataFacadeT * facade ) + explicit NearestPlugin(DataFacadeT * facade ) : facade(facade), descriptor_string("nearest") diff --git a/Plugins/TimestampPlugin.h b/Plugins/TimestampPlugin.h index 7d46a9c1a..df4233cd4 100644 --- a/Plugins/TimestampPlugin.h +++ b/Plugins/TimestampPlugin.h @@ -33,7 +33,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. template class TimestampPlugin : public BasePlugin { public: - TimestampPlugin(const DataFacadeT * facade) + explicit TimestampPlugin(const DataFacadeT * facade) : facade(facade), descriptor_string("timestamp") { } const std::string & GetDescriptor() const { return descriptor_string; } diff --git a/Plugins/ViaRoutePlugin.h b/Plugins/ViaRoutePlugin.h index 6f2880e04..5f25f20bc 100644 --- a/Plugins/ViaRoutePlugin.h +++ b/Plugins/ViaRoutePlugin.h @@ -53,7 +53,7 @@ private: SearchEngine * search_engine_ptr; public: - ViaRoutePlugin(DataFacadeT * facade) + explicit ViaRoutePlugin(DataFacadeT * facade) : descriptor_string("viaroute"), facade(facade) diff --git a/Server/DataStructures/InternalDataFacade.h b/Server/DataStructures/InternalDataFacade.h index c6ae80832..b8a552cf3 100644 --- a/Server/DataStructures/InternalDataFacade.h +++ b/Server/DataStructures/InternalDataFacade.h @@ -207,7 +207,7 @@ public: delete m_static_rtree; } - InternalDataFacade( const ServerPaths & server_paths ) { + explicit InternalDataFacade( const ServerPaths & server_paths ) { //generate paths of data files if( server_paths.find("hsgrdata") == server_paths.end() ) { throw OSRMException("no hsgr file given in ini file"); diff --git a/Server/Server.h b/Server/Server.h index 96e07749b..bdb1ccc21 100644 --- a/Server/Server.h +++ b/Server/Server.h @@ -33,6 +33,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include #include #include #include @@ -72,7 +73,7 @@ public: void Run() { std::vector > threads; for (unsigned i = 0; i < threadPoolSize; ++i) { - boost::shared_ptr thread(new boost::thread(boost::bind(&boost::asio::io_service::run, &ioService))); + boost::shared_ptr thread = boost::make_shared(boost::bind(&boost::asio::io_service::run, &ioService)); threads.push_back(thread); } for (unsigned i = 0; i < threads.size(); ++i) From c23575786c60e8442c00f015e5419d807cad1b9f Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 16 Apr 2014 18:13:59 +0200 Subject: [PATCH 18/27] Revert "fix resource leak, coverity issue 1198844" This reverts commit 3a1a51ac4676dde4170648e56778a5ec50a70279. --- Algorithms/StronglyConnectedComponents.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Algorithms/StronglyConnectedComponents.h b/Algorithms/StronglyConnectedComponents.h index be6ddb7e7..e0245ef38 100644 --- a/Algorithms/StronglyConnectedComponents.h +++ b/Algorithms/StronglyConnectedComponents.h @@ -145,7 +145,7 @@ public: if(restriction_iterator == m_restriction_map.end()) { index = m_restriction_bucket_list.size(); m_restriction_bucket_list.resize(index+1); - restriction_iterator->second = index; + m_restriction_map[restrictionSource] = index; } else { index = restriction_iterator->second; //Map already contains an is_only_*-restriction From 2ef37ee7987d058c023c8010ba51aa60002480e2 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Thu, 17 Apr 2014 10:30:15 +0200 Subject: [PATCH 19/27] make single parameter c'tor explicit --- Plugins/LocatePlugin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Plugins/LocatePlugin.h b/Plugins/LocatePlugin.h index 7994b0edf..dca262f97 100644 --- a/Plugins/LocatePlugin.h +++ b/Plugins/LocatePlugin.h @@ -36,7 +36,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. template class LocatePlugin : public BasePlugin { public: - LocatePlugin(DataFacadeT * facade) + explicit LocatePlugin(DataFacadeT * facade) : descriptor_string("locate"), facade(facade) From a5ebdb92439af0ee3653f043b7bde4fa99b3ee6d Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Thu, 17 Apr 2014 11:05:26 +0200 Subject: [PATCH 20/27] use insert(.) on unordered map instead of access operator, potential resource leak --- Algorithms/StronglyConnectedComponents.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Algorithms/StronglyConnectedComponents.h b/Algorithms/StronglyConnectedComponents.h index e0245ef38..e94ea7ac3 100644 --- a/Algorithms/StronglyConnectedComponents.h +++ b/Algorithms/StronglyConnectedComponents.h @@ -96,7 +96,7 @@ private: typedef std::pair RestrictionSource; typedef std::pair restriction_target; typedef std::vector EmanatingRestrictionsVector; - typedef boost::unordered_map RestrictionMap; + typedef boost::unordered_map RestrictionMap; std::vector m_coordinate_list; std::vector m_restriction_bucket_list; @@ -145,7 +145,7 @@ public: if(restriction_iterator == m_restriction_map.end()) { index = m_restriction_bucket_list.size(); m_restriction_bucket_list.resize(index+1); - m_restriction_map[restrictionSource] = index; + m_restriction_map.insert(std::make_pair(restrictionSource, index)); } else { index = restriction_iterator->second; //Map already contains an is_only_*-restriction @@ -358,6 +358,7 @@ public: "identified: " << component_size_vector.size() << " many components, marking small components"; + // TODO/C++11: prime candidate for lambda function unsigned size_one_counter = 0; for(unsigned i = 0, end = component_size_vector.size(); i < end; ++i){ if(1 == component_size_vector[i]) { From 727a29600d9cf21fac4784523e3fb4c4cc233406 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Sat, 19 Apr 2014 13:19:48 +0200 Subject: [PATCH 21/27] better error messages for extractor Previously extractor would just crash because of unhandled exceptions. Now it displays meaningful error messages. --- extractor.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/extractor.cpp b/extractor.cpp index deae476a9..e57901a4b 100644 --- a/extractor.cpp +++ b/extractor.cpp @@ -132,6 +132,16 @@ int main (int argc, char *argv[]) { return -1; } + if(!boost::filesystem::is_regular_file(input_path)) { + SimpleLogger().Write(logWARNING) << "Input file " << input_path.c_str() << " not found!"; + return -1; + } + + if(!boost::filesystem::is_regular_file(profile_path)) { + SimpleLogger().Write(logWARNING) << "Profile " << profile_path.c_str() << " not found!"; + return -1; + } + SimpleLogger().Write() << "Input file: " << input_path.filename().string(); SimpleLogger().Write() << "Profile: " << profile_path.filename().string(); SimpleLogger().Write() << "Threads: " << requested_num_threads; From e5adaf974d199ba46ebfc2be02bd810886879041 Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Mon, 21 Apr 2014 15:24:42 +0200 Subject: [PATCH 22/27] cuke: test options for osrm-extract and osrm-prepare --- .../{version.feature => extract.feature} | 8 +-- features/options/extract/files.feature | 31 ++++++++ features/options/extract/help.feature | 48 +++++++++++++ features/options/extract/invalid.feature | 13 ++++ features/options/extract/version.feature | 22 ++++++ features/options/prepare/files.feature | 35 +++++++++ features/options/prepare/help.feature | 51 +++++++++++++ features/options/prepare/invalid.feature | 13 ++++ features/options/prepare/version.feature | 22 ++++++ features/options/{ => routed}/files.feature | 10 +-- features/options/{ => routed}/help.feature | 13 ++-- features/options/{ => routed}/invalid.feature | 8 +-- features/options/routed/version.feature | 22 ++++++ features/step_definitions/data.rb | 25 +++++++ features/step_definitions/options.rb | 30 ++++---- features/step_definitions/requests.rb | 10 +-- features/support/data.rb | 72 +++++++++---------- features/support/env.rb | 20 ++++++ features/support/run.rb | 18 +++++ features/testbot/bad.feature | 2 +- features/testbot/protobuffer.feature | 2 +- 21 files changed, 392 insertions(+), 83 deletions(-) rename features/options/{version.feature => extract.feature} (82%) create mode 100644 features/options/extract/files.feature create mode 100644 features/options/extract/help.feature create mode 100644 features/options/extract/invalid.feature create mode 100644 features/options/extract/version.feature create mode 100644 features/options/prepare/files.feature create mode 100644 features/options/prepare/help.feature create mode 100644 features/options/prepare/invalid.feature create mode 100644 features/options/prepare/version.feature rename features/options/{ => routed}/files.feature (83%) rename features/options/{ => routed}/help.feature (89%) rename features/options/{ => routed}/invalid.feature (75%) create mode 100644 features/options/routed/version.feature create mode 100644 features/support/run.rb diff --git a/features/options/version.feature b/features/options/extract.feature similarity index 82% rename from features/options/version.feature rename to features/options/extract.feature index 9ad933924..1582888b7 100644 --- a/features/options/version.feature +++ b/features/options/extract.feature @@ -1,5 +1,5 @@ -@routing @options -Feature: Command line options: version +@extract @options +Feature: osrm-extract command line options: version # the regex will match these two formats: # v0.3.7.0 # this is the normal format when you build from a git clone # -128-NOTFOUND # if you build from a shallow clone (used on Travis) @@ -7,14 +7,14 @@ Feature: Command line options: version Background: Given the profile "testbot" - Scenario: Version, short + Scenario: osrm-extract - Version, short When I run "osrm-routed --v" Then stderr should be empty And stdout should contain 1 line And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ And it should exit with code 0 - Scenario: Version, long + Scenario: osrm-extract - Version, long When I run "osrm-routed --version" Then stderr should be empty And stdout should contain 1 line diff --git a/features/options/extract/files.feature b/features/options/extract/files.feature new file mode 100644 index 000000000..9b7a05408 --- /dev/null +++ b/features/options/extract/files.feature @@ -0,0 +1,31 @@ +@extract @options @files +Feature: osrm-extract command line options: files +# expansions: +# {base} => path to current input file +# {profile} => path to current profile script + + Background: + Given the profile "testbot" + And the node map + | a | b | + And the ways + | nodes | + | ab | + And the data has been saved to disk + + Scenario: osrm-extract - Passing base file + When I run "osrm-extract {base}.osm --profile {profile}" + Then stderr should be empty + And it should exit with code 0 + + Scenario: osrm-extract - Order of options should not matter + When I run "osrm-extract --profile {profile} {base}.osm" + Then stderr should be empty + And it should exit with code 0 + + @todo + Scenario: osrm-extract - Missing input file + When I run "osrm-extract over-the-rainbow.osrm --profile {profile}" + And stderr should contain "over-the-rainbow.osrm" + And stderr should contain "not found" + And it should exit with code 1 diff --git a/features/options/extract/help.feature b/features/options/extract/help.feature new file mode 100644 index 000000000..3a40c108d --- /dev/null +++ b/features/options/extract/help.feature @@ -0,0 +1,48 @@ +@extract @options @help +Feature: osrm-extract command line options: help + + Background: + Given the profile "testbot" + + @todo + Scenario: osrm-extract - Help should be shown when no options are passed + When I run "osrm-extract" + Then stderr should be empty + And stdout should contain "osrm-extract [options]:" + And stdout should contain "Options:" + And stdout should contain "--version" + And stdout should contain "--help" + And stdout should contain "--config" + And stdout should contain "Configuration:" + And stdout should contain "--profile" + And stdout should contain "--threads" + And stdout should contain 12 lines + And it should exit with code 0 + + Scenario: osrm-extract - Help, short + When I run "osrm-extract -h" + Then stderr should be empty + And stdout should contain "osrm-extract [options]:" + And stdout should contain "Options:" + And stdout should contain "--version" + And stdout should contain "--help" + And stdout should contain "--config" + And stdout should contain "Configuration:" + And stdout should contain "--profile" + And stdout should contain "--threads" + And stdout should contain 12 lines + And it should exit with code 0 + + Scenario: osrm-extract - Help, long + When I run "osrm-extract --help" + Then stderr should be empty + And stdout should contain "osrm-extract [options]:" + And stdout should contain "Options:" + And stdout should contain "--version" + And stdout should contain "--help" + And stdout should contain "--config" + And stdout should contain "Configuration:" + And stdout should contain "--profile" + And stdout should contain "--threads" + And stdout should contain 12 lines + And it should exit with code 0 \ No newline at end of file diff --git a/features/options/extract/invalid.feature b/features/options/extract/invalid.feature new file mode 100644 index 000000000..2fd4b7509 --- /dev/null +++ b/features/options/extract/invalid.feature @@ -0,0 +1,13 @@ +@extract @options @invalid +Feature: osrm-extract command line options: invalid options + + Background: + Given the profile "testbot" + + @todo + Scenario: osrm-extract - Non-existing option + When I run "osrm-extract --fly-me-to-the-moon" + Then stdout should be empty + And stderr should contain "exception" + And stderr should contain "fly-me-to-the-moon" + And it should exit with code 1 diff --git a/features/options/extract/version.feature b/features/options/extract/version.feature new file mode 100644 index 000000000..0dd5f6588 --- /dev/null +++ b/features/options/extract/version.feature @@ -0,0 +1,22 @@ +@extract @options @version +Feature: osrm-extract command line options: version +# the regex will match these two formats: +# v0.3.7.0 # this is the normal format when you build from a git clone +# -128-NOTFOUND # if you build from a shallow clone (used on Travis) + + Background: + Given the profile "testbot" + + Scenario: osrm-extract - Version, short + When I run "osrm-extract --v" + Then stderr should be empty + And stdout should contain 1 line + And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ + And it should exit with code 0 + + Scenario: osrm-extract - Version, long + When I run "osrm-extract --version" + Then stderr should be empty + And stdout should contain 1 line + And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ + And it should exit with code 0 diff --git a/features/options/prepare/files.feature b/features/options/prepare/files.feature new file mode 100644 index 000000000..21fd390c7 --- /dev/null +++ b/features/options/prepare/files.feature @@ -0,0 +1,35 @@ +@prepare @options @files +Feature: osrm-prepare command line options: files +# expansions: +# {base} => path to current input file +# {profile} => path to current profile script + + Background: + Given the profile "testbot" + And the node map + | a | b | + And the ways + | nodes | + | ab | + And the data has been extracted + + Scenario: osrm-prepare - Passing base file + When I run "osrm-extract {base}.osm --profile {profile}" + Then stderr should be empty + And it should exit with code 0 + When I run "osrm-prepare {base}.osrm --profile {profile}" + Then stderr should be empty + And it should exit with code 0 + + @todo + Scenario: osrm-prepare - Order of options should not matter + When I run "osrm-prepare --profile {profile} {base}.osm" + Then stderr should be empty + And it should exit with code 0 + + @todo + Scenario: osrm-prepare - Missing input file + When I run "osrm-prepare over-the-rainbow.osrm --profile {profile}" + And stderr should contain "over-the-rainbow.osrm" + And stderr should contain "not found" + And it should exit with code 1 diff --git a/features/options/prepare/help.feature b/features/options/prepare/help.feature new file mode 100644 index 000000000..ea7ae1850 --- /dev/null +++ b/features/options/prepare/help.feature @@ -0,0 +1,51 @@ +@prepare @options @help +Feature: osrm-prepare command line options: help + + Background: + Given the profile "testbot" + + @todo + Scenario: osrm-prepare - Help should be shown when no options are passed + When I run "osrm-prepare" + Then stderr should be empty + And stdout should contain "osrm-prepare [options]:" + And stdout should contain "Options:" + And stdout should contain "--version" + And stdout should contain "--help" + And stdout should contain "--config" + And stdout should contain "Configuration:" + And stdout should contain "--restrictions" + And stdout should contain "--profile" + And stdout should contain "--threads" + And stdout should contain 15 lines + And it should exit with code 0 + + Scenario: osrm-prepare - Help, short + When I run "osrm-prepare -h" + Then stderr should be empty + And stdout should contain "osrm-prepare [options]:" + And stdout should contain "Options:" + And stdout should contain "--version" + And stdout should contain "--help" + And stdout should contain "--config" + And stdout should contain "Configuration:" + And stdout should contain "--restrictions" + And stdout should contain "--profile" + And stdout should contain "--threads" + And stdout should contain 15 lines + And it should exit with code 0 + + Scenario: osrm-prepare - Help, long + When I run "osrm-prepare --help" + Then stderr should be empty + And stdout should contain "osrm-prepare [options]:" + And stdout should contain "Options:" + And stdout should contain "--version" + And stdout should contain "--help" + And stdout should contain "--config" + And stdout should contain "Configuration:" + And stdout should contain "--restrictions" + And stdout should contain "--profile" + And stdout should contain "--threads" + And stdout should contain 15 lines + And it should exit with code 0 \ No newline at end of file diff --git a/features/options/prepare/invalid.feature b/features/options/prepare/invalid.feature new file mode 100644 index 000000000..a17606613 --- /dev/null +++ b/features/options/prepare/invalid.feature @@ -0,0 +1,13 @@ +@prepare @options @invalid +Feature: osrm-prepare command line options: invalid options + + Background: + Given the profile "testbot" + + @todo + Scenario: osrm-prepare - Non-existing option + When I run "osrm-prepare --fly-me-to-the-moon" + Then stdout should be empty + And stderr should contain "exception" + And stderr should contain "fly-me-to-the-moon" + And it should exit with code 1 \ No newline at end of file diff --git a/features/options/prepare/version.feature b/features/options/prepare/version.feature new file mode 100644 index 000000000..7a821c626 --- /dev/null +++ b/features/options/prepare/version.feature @@ -0,0 +1,22 @@ +@prepare @options @version +Feature: osrm-prepare command line options: version +# the regex will match these two formats: +# v0.3.7.0 # this is the normal format when you build from a git clone +# -128-NOTFOUND # if you build from a shallow clone (used on Travis) + + Background: + Given the profile "testbot" + + Scenario: osrm-prepare - Version, short + When I run "osrm-prepare --v" + Then stderr should be empty + And stdout should contain 1 line + And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ + And it should exit with code 0 + + Scenario: osrm-prepare - Version, long + When I run "osrm-prepare --version" + Then stderr should be empty + And stdout should contain 1 line + And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ + And it should exit with code 0 diff --git a/features/options/files.feature b/features/options/routed/files.feature similarity index 83% rename from features/options/files.feature rename to features/options/routed/files.feature index 41252238e..716b2a4d8 100644 --- a/features/options/files.feature +++ b/features/options/routed/files.feature @@ -1,5 +1,5 @@ -@routing @options @files -Feature: Command line options: files +@routed @options @files +Feature: osrm-routed command line options: files # Normally when launching osrm-routed, it will keep running as a server until it's shut down. # For testing program options, the --trial option is used, which causes osrm-routed to quit # immediately after initialization. This makes testing easier and faster. @@ -14,9 +14,9 @@ Feature: Command line options: files And the ways | nodes | | ab | - And I preprocess data + And the data has been prepared - Scenario: Passing base file + Scenario: osrm-routed - Passing base file When I run "osrm-routed {base}.osrm --trial" Then stdout should contain /^\[info\] starting up engines/ And stdout should contain /\d{1,2}\.\d{1,2}\.\d{1,2}/ @@ -24,4 +24,4 @@ Feature: Command line options: files And stdout should contain /^\[info\] loaded plugin: viaroute/ And stdout should contain /^\[info\] trial run/ And stdout should contain /^\[info\] shutdown completed/ - And it should exit with code 0 + And it should exit with code 0 \ No newline at end of file diff --git a/features/options/help.feature b/features/options/routed/help.feature similarity index 89% rename from features/options/help.feature rename to features/options/routed/help.feature index 40ec0177e..3feb95746 100644 --- a/features/options/help.feature +++ b/features/options/routed/help.feature @@ -1,10 +1,10 @@ -@routing @options -Feature: Command line options: help +@routed @options @help +Feature: osrm-routed command line options: help Background: Given the profile "testbot" - Scenario: Help should be shown when no options are passed + Scenario: osrm-routed - Help should be shown when no options are passed When I run "osrm-routed" Then stderr should be empty And stdout should contain "osrm-routed []:" @@ -25,9 +25,10 @@ Feature: Command line options: help And stdout should contain "--port" And stdout should contain "--threads" And stdout should contain "--sharedmemory" + And stdout should contain 21 lines And it should exit with code 0 - Scenario: Help, short + Scenario: osrm-routed - Help, short When I run "osrm-routed -h" Then stderr should be empty And stdout should contain "osrm-routed []:" @@ -48,9 +49,10 @@ Feature: Command line options: help And stdout should contain "--port" And stdout should contain "--threads" And stdout should contain "--sharedmemory" + And stdout should contain 21 lines And it should exit with code 0 - Scenario: Help, long + Scenario: osrm-routed - Help, long When I run "osrm-routed --help" Then stderr should be empty And stdout should contain "osrm-routed []:" @@ -71,4 +73,5 @@ Feature: Command line options: help And stdout should contain "--port" And stdout should contain "--threads" And stdout should contain "--sharedmemory" + And stdout should contain 21 lines And it should exit with code 0 \ No newline at end of file diff --git a/features/options/invalid.feature b/features/options/routed/invalid.feature similarity index 75% rename from features/options/invalid.feature rename to features/options/routed/invalid.feature index b1ea4b420..a25a039c0 100644 --- a/features/options/invalid.feature +++ b/features/options/routed/invalid.feature @@ -1,17 +1,17 @@ -@routing @options -Feature: Command line options: invalid options +@routed @options @invalid +Feature: osrm-routed command line options: invalid options Background: Given the profile "testbot" - Scenario: Non-existing option + Scenario: osrm-routed - Non-existing option When I run "osrm-routed --fly-me-to-the-moon" Then stdout should be empty And stderr should contain "exception" And stderr should contain "fly-me-to-the-moon" And it should exit with code 1 - Scenario: Missing file + Scenario: osrm-routed - Missing file When I run "osrm-routed over-the-rainbow.osrm" Then stdout should contain "over-the-rainbow.osrm" And stderr should contain "exception" diff --git a/features/options/routed/version.feature b/features/options/routed/version.feature new file mode 100644 index 000000000..b544e36e6 --- /dev/null +++ b/features/options/routed/version.feature @@ -0,0 +1,22 @@ +@routed @options @version +Feature: osrm-routed command line options: version +# the regex will match these two formats: +# v0.3.7.0 # this is the normal format when you build from a git clone +# -128-NOTFOUND # if you build from a shallow clone (used on Travis) + + Background: + Given the profile "testbot" + + Scenario: osrm-routed - Version, short + When I run "osrm-routed --v" + Then stderr should be empty + And stdout should contain 1 line + And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ + And it should exit with code 0 + + Scenario: osrm-routed - Version, long + When I run "osrm-routed --version" + Then stderr should be empty + And stdout should contain 1 line + And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ + And it should exit with code 0 diff --git a/features/step_definitions/data.rb b/features/step_definitions/data.rb index d6cea41da..e0985a4b1 100644 --- a/features/step_definitions/data.rb +++ b/features/step_definitions/data.rb @@ -133,3 +133,28 @@ Given /^the input file ([^"]*)$/ do |file| raise "*** Input file must in .osm format" unless File.extname(file)=='.osm' @osm_str = File.read file end + +Given /^the data has been saved to disk$/ do + begin + write_input_data + rescue OSRMError => e + @process_error = e + end +end + +Given /^the data has been extracted$/ do + begin + write_input_data + extract_data unless extracted? + rescue OSRMError => e + @process_error = e + end +end + +Given /^the data has been prepared$/ do + begin + reprocess + rescue OSRMError => e + @process_error = e + end +end diff --git a/features/step_definitions/options.rb b/features/step_definitions/options.rb index 6b349f8b5..ac3bc9983 100644 --- a/features/step_definitions/options.rb +++ b/features/step_definitions/options.rb @@ -1,25 +1,19 @@ When(/^I run "osrm\-routed\s?(.*?)"$/) do |options| - Dir.chdir TEST_FOLDER do - if options.include? '{base}' - # expand {base} to base path of preprocessed data file - raise "*** Cannot expand {base} without a preprocessed file." unless @osm_file - options_expanded = options.gsub "{base}", "#{@osm_file}" - else - options_expanded = options - end - - begin - Timeout.timeout(1) do - @stdout = `#{BIN_PATH}/osrm-routed #{options_expanded} 2>error.log` - @stderr = File.read 'error.log' - @exit_code = $?.exitstatus - end - rescue Timeout::Error - raise "*** osrm-routed didn't quit. Maybe the --trial option wasn't used?" - end + begin + Timeout.timeout(1) { run_bin 'osrm-routed', options } + rescue Timeout::Error + raise "*** osrm-routed didn't quit. Maybe the --trial option wasn't used?" end end +When(/^I run "osrm\-extract\s?(.*?)"$/) do |options| + run_bin 'osrm-extract', options +end + +When(/^I run "osrm\-prepare\s?(.*?)"$/) do |options| + run_bin 'osrm-prepare', options +end + Then /^it should exit with code (\d+)$/ do |code| @exit_code.should == code.to_i end diff --git a/features/step_definitions/requests.rb b/features/step_definitions/requests.rb index e6bdcae84..b40551f32 100644 --- a/features/step_definitions/requests.rb +++ b/features/step_definitions/requests.rb @@ -29,16 +29,8 @@ Then /^response should be a well-formed route$/ do @json['via_indices'].class.should == Array end -When /^I preprocess data$/ do - begin - reprocess - rescue OSRMError => e - @process_error = e - end -end - Then /^"([^"]*)" should return code (\d+)$/ do |binary, code| @process_error.is_a?(OSRMError).should == true @process_error.process.should == binary @process_error.code.to_i.should == code.to_i -end +end \ No newline at end of file diff --git a/features/support/data.rb b/features/support/data.rb index 1d9522994..58d5de759 100644 --- a/features/support/data.rb +++ b/features/support/data.rb @@ -2,20 +2,6 @@ require 'OSM/objects' #osmlib gem require 'OSM/Database' require 'builder' -OSM_USER = 'osrm' -OSM_GENERATOR = 'osrm-test' -OSM_UID = 1 -TEST_FOLDER = 'test' -DATA_FOLDER = 'cache' -OSM_TIMESTAMP = '2000-00-00T00:00:00Z' -DEFAULT_SPEEDPROFILE = 'bicycle' -WAY_SPACING = 100 -DEFAULT_GRID_SIZE = 100 #meters -PROFILES_PATH = '../profiles' -BIN_PATH = '../build' -DEFAULT_INPUT_FORMAT = 'osm' -DEFAULT_ORIGIN = [1,1] - class Location attr_accessor :lon,:lat @@ -252,30 +238,44 @@ def write_timestamp File.open( "#{@osm_file}.osrm.timestamp", 'w') {|f| f.write(OSM_TIMESTAMP) } end -def reprocess +def pbf? + input_format=='pbf' +end +def write_input_data Dir.chdir TEST_FOLDER do - use_pbf = (input_format=='pbf') write_osm write_timestamp - convert_osm_to_pbf if use_pbf - unless extracted? - log_preprocess_info - log "== Extracting #{@osm_file}.osm...", :preprocess - unless system "#{BIN_PATH}/osrm-extract #{@osm_file}.osm#{'.pbf' if use_pbf} --profile #{PROFILES_PATH}/#{@profile}.lua 1>>#{PREPROCESS_LOG_FILE} 2>>#{PREPROCESS_LOG_FILE}" - log "*** Exited with code #{$?.exitstatus}.", :preprocess - raise ExtractError.new $?.exitstatus, "osrm-extract exited with code #{$?.exitstatus}." - end - log '', :preprocess - end - unless prepared? - log_preprocess_info - log "== Preparing #{@osm_file}.osm...", :preprocess - unless system "#{BIN_PATH}/osrm-prepare #{@osm_file}.osrm --profile #{PROFILES_PATH}/#{@profile}.lua 1>>#{PREPROCESS_LOG_FILE} 2>>#{PREPROCESS_LOG_FILE}" - log "*** Exited with code #{$?.exitstatus}.", :preprocess - raise PrepareError.new $?.exitstatus, "osrm-prepare exited with code #{$?.exitstatus}." - end - log '', :preprocess - end - log_preprocess_done + convert_osm_to_pbf if pbf? end end + +def extract_data + Dir.chdir TEST_FOLDER do + log_preprocess_info + log "== Extracting #{@osm_file}.osm...", :preprocess + unless system "#{BIN_PATH}/osrm-extract #{@osm_file}.osm#{'.pbf' if pbf?} --profile #{PROFILES_PATH}/#{@profile}.lua 1>>#{PREPROCESS_LOG_FILE} 2>>#{PREPROCESS_LOG_FILE}" + log "*** Exited with code #{$?.exitstatus}.", :preprocess + raise ExtractError.new $?.exitstatus, "osrm-extract exited with code #{$?.exitstatus}." + end + log '', :preprocess + end +end + +def prepare_data + Dir.chdir TEST_FOLDER do + log_preprocess_info + log "== Preparing #{@osm_file}.osm...", :preprocess + unless system "#{BIN_PATH}/osrm-prepare #{@osm_file}.osrm --profile #{PROFILES_PATH}/#{@profile}.lua 1>>#{PREPROCESS_LOG_FILE} 2>>#{PREPROCESS_LOG_FILE}" + log "*** Exited with code #{$?.exitstatus}.", :preprocess + raise PrepareError.new $?.exitstatus, "osrm-prepare exited with code #{$?.exitstatus}." + end + log '', :preprocess + end +end + +def reprocess + write_input_data + extract_data unless extracted? + prepare_data unless prepared? + log_preprocess_done +end diff --git a/features/support/env.rb b/features/support/env.rb index 2cbc4e46d..b1eeb64b6 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -3,6 +3,22 @@ require 'rspec/expectations' DEFAULT_PORT = 5000 DEFAULT_TIMEOUT = 2 +ROOT_FOLDER = Dir.pwd +OSM_USER = 'osrm' +OSM_GENERATOR = 'osrm-test' +OSM_UID = 1 +TEST_FOLDER = File.join ROOT_FOLDER, 'test' +DATA_FOLDER = 'cache' +OSM_TIMESTAMP = '2000-00-00T00:00:00Z' +DEFAULT_SPEEDPROFILE = 'bicycle' +WAY_SPACING = 100 +DEFAULT_GRID_SIZE = 100 #meters +PROFILES_PATH = File.join ROOT_FOLDER, 'profiles' +BIN_PATH = File.join ROOT_FOLDER, 'build' +DEFAULT_INPUT_FORMAT = 'osm' +DEFAULT_ORIGIN = [1,1] + + puts "Ruby version #{RUBY_VERSION}" unless RUBY_VERSION.to_f >= 1.9 raise "*** Please upgrade to Ruby 1.9.x to run the OSRM cucumber tests" @@ -24,6 +40,10 @@ else puts "Using default timeout #{OSRM_TIMEOUT}" end +unless File.exists? TEST_FOLDER + raise "*** Test folder #{TEST_FOLDER} doesn't exist." +end + AfterConfiguration do |config| clear_log_files diff --git a/features/support/run.rb b/features/support/run.rb new file mode 100644 index 000000000..236c873d9 --- /dev/null +++ b/features/support/run.rb @@ -0,0 +1,18 @@ +def run_bin bin, options + Dir.chdir TEST_FOLDER do + opt = options.dup + + if opt.include? '{base}' + raise "*** {base} is missing" unless @osm_file + opt.gsub! "{base}", "#{@osm_file}" + end + + if opt.include? '{profile}' + opt.gsub! "{profile}", "#{PROFILES_PATH}/#{@profile}.lua" + end + + @stdout = `#{BIN_PATH}/#{bin} #{opt} 2>error.log` + @stderr = File.read 'error.log' + @exit_code = $?.exitstatus + end +end \ No newline at end of file diff --git a/features/testbot/bad.feature b/features/testbot/bad.feature index 98f5dbef2..f28b07295 100644 --- a/features/testbot/bad.feature +++ b/features/testbot/bad.feature @@ -11,7 +11,7 @@ Feature: Handle bad data in a graceful manner Given the ways | nodes | - When I preprocess data + When the data has been prepared Then "osrm-extract" should return code 255 Scenario: Only dead-end oneways diff --git a/features/testbot/protobuffer.feature b/features/testbot/protobuffer.feature index 016a233be..7d8329ad9 100644 --- a/features/testbot/protobuffer.feature +++ b/features/testbot/protobuffer.feature @@ -87,7 +87,7 @@ Feature: Importing protobuffer (.pbf) format Given the ways | nodes | - When I preprocess data + When the data has been prepared Then "osrm-extract" should return code 255 From c1eb00f6d53d5dfada3bb951713a7e867fcc0c31 Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Mon, 21 Apr 2014 16:07:03 +0200 Subject: [PATCH 23/27] cuke: fix options test, should use osrm file, not osm --- features/options/prepare/files.feature | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/features/options/prepare/files.feature b/features/options/prepare/files.feature index 21fd390c7..25949c52e 100644 --- a/features/options/prepare/files.feature +++ b/features/options/prepare/files.feature @@ -14,16 +14,12 @@ Feature: osrm-prepare command line options: files And the data has been extracted Scenario: osrm-prepare - Passing base file - When I run "osrm-extract {base}.osm --profile {profile}" - Then stderr should be empty - And it should exit with code 0 When I run "osrm-prepare {base}.osrm --profile {profile}" Then stderr should be empty And it should exit with code 0 - @todo Scenario: osrm-prepare - Order of options should not matter - When I run "osrm-prepare --profile {profile} {base}.osm" + When I run "osrm-prepare --profile {profile} {base}.osrm" Then stderr should be empty And it should exit with code 0 From 14ad02777f8e5f01335e61f18c3e04c7c3c01b0b Mon Sep 17 00:00:00 2001 From: Emil Tin Date: Mon, 21 Apr 2014 17:18:30 +0200 Subject: [PATCH 24/27] remove left-over feature file --- features/options/extract.feature | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 features/options/extract.feature diff --git a/features/options/extract.feature b/features/options/extract.feature deleted file mode 100644 index 1582888b7..000000000 --- a/features/options/extract.feature +++ /dev/null @@ -1,22 +0,0 @@ -@extract @options -Feature: osrm-extract command line options: version -# the regex will match these two formats: -# v0.3.7.0 # this is the normal format when you build from a git clone -# -128-NOTFOUND # if you build from a shallow clone (used on Travis) - - Background: - Given the profile "testbot" - - Scenario: osrm-extract - Version, short - When I run "osrm-routed --v" - Then stderr should be empty - And stdout should contain 1 line - And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ - And it should exit with code 0 - - Scenario: osrm-extract - Version, long - When I run "osrm-routed --version" - Then stderr should be empty - And stdout should contain 1 line - And stdout should contain /(v\d{1,2}\.\d{1,2}\.\d{1,2}|\w*-\d+-\w+)/ - And it should exit with code 0 From 853f6012d58c33669cbc9afcbf442eed8f7cd180 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Mon, 21 Apr 2014 17:20:15 +0200 Subject: [PATCH 25/27] implementing option tests marked @todo --- extractor.cpp | 17 ++++---- features/options/extract/files.feature | 3 +- features/options/extract/help.feature | 5 +-- features/options/extract/invalid.feature | 3 +- features/options/prepare/files.feature | 7 ++- features/options/prepare/help.feature | 5 +-- features/options/prepare/invalid.feature | 7 ++- prepare.cpp | 54 ++++++++++++------------ 8 files changed, 49 insertions(+), 52 deletions(-) diff --git a/extractor.cpp b/extractor.cpp index e57901a4b..70ff15ced 100644 --- a/extractor.cpp +++ b/extractor.cpp @@ -122,24 +122,23 @@ int main (int argc, char *argv[]) { } if(!option_variables.count("input")) { - SimpleLogger().Write(logWARNING) << "No input file specified"; SimpleLogger().Write() << visible_options; - return -1; + return 0; } if(1 > requested_num_threads) { SimpleLogger().Write(logWARNING) << "Number of threads must be 1 or larger"; - return -1; + return 1; } if(!boost::filesystem::is_regular_file(input_path)) { SimpleLogger().Write(logWARNING) << "Input file " << input_path.c_str() << " not found!"; - return -1; + return 1; } if(!boost::filesystem::is_regular_file(profile_path)) { SimpleLogger().Write(logWARNING) << "Profile " << profile_path.c_str() << " not found!"; - return -1; + return 1; } SimpleLogger().Write() << "Input file: " << input_path.filename().string(); @@ -205,7 +204,7 @@ int main (int argc, char *argv[]) { if( externalMemory.all_edges_list.empty() ) { SimpleLogger().Write(logWARNING) << "The input data is empty, exiting."; - return -1; + return 1; } externalMemory.PrepareData(output_file_name, restrictionsFileName); @@ -222,13 +221,13 @@ int main (int argc, char *argv[]) { } catch(boost::program_options::too_many_positional_options_error& e) { SimpleLogger().Write(logWARNING) << "Only one input file can be specified"; - return -1; + return 1; } catch(boost::program_options::error& e) { SimpleLogger().Write(logWARNING) << e.what(); - return -1; + return 1; } catch(std::exception & e) { SimpleLogger().Write(logWARNING) << "Unhandled exception: " << e.what(); - return -1; + return 1; } return 0; } diff --git a/features/options/extract/files.feature b/features/options/extract/files.feature index 9b7a05408..3061263e9 100644 --- a/features/options/extract/files.feature +++ b/features/options/extract/files.feature @@ -22,8 +22,7 @@ Feature: osrm-extract command line options: files When I run "osrm-extract --profile {profile} {base}.osm" Then stderr should be empty And it should exit with code 0 - - @todo + Scenario: osrm-extract - Missing input file When I run "osrm-extract over-the-rainbow.osrm --profile {profile}" And stderr should contain "over-the-rainbow.osrm" diff --git a/features/options/extract/help.feature b/features/options/extract/help.feature index 3a40c108d..3215e6aef 100644 --- a/features/options/extract/help.feature +++ b/features/options/extract/help.feature @@ -3,8 +3,7 @@ Feature: osrm-extract command line options: help Background: Given the profile "testbot" - - @todo + Scenario: osrm-extract - Help should be shown when no options are passed When I run "osrm-extract" Then stderr should be empty @@ -45,4 +44,4 @@ Feature: osrm-extract command line options: help And stdout should contain "--profile" And stdout should contain "--threads" And stdout should contain 12 lines - And it should exit with code 0 \ No newline at end of file + And it should exit with code 0 diff --git a/features/options/extract/invalid.feature b/features/options/extract/invalid.feature index 2fd4b7509..3ef302ba0 100644 --- a/features/options/extract/invalid.feature +++ b/features/options/extract/invalid.feature @@ -3,8 +3,7 @@ Feature: osrm-extract command line options: invalid options Background: Given the profile "testbot" - - @todo + Scenario: osrm-extract - Non-existing option When I run "osrm-extract --fly-me-to-the-moon" Then stdout should be empty diff --git a/features/options/prepare/files.feature b/features/options/prepare/files.feature index 25949c52e..fb078d0f1 100644 --- a/features/options/prepare/files.feature +++ b/features/options/prepare/files.feature @@ -17,13 +17,16 @@ Feature: osrm-prepare command line options: files When I run "osrm-prepare {base}.osrm --profile {profile}" Then stderr should be empty And it should exit with code 0 +<<<<<<< Updated upstream +======= + +>>>>>>> Stashed changes Scenario: osrm-prepare - Order of options should not matter When I run "osrm-prepare --profile {profile} {base}.osrm" Then stderr should be empty And it should exit with code 0 - - @todo + Scenario: osrm-prepare - Missing input file When I run "osrm-prepare over-the-rainbow.osrm --profile {profile}" And stderr should contain "over-the-rainbow.osrm" diff --git a/features/options/prepare/help.feature b/features/options/prepare/help.feature index ea7ae1850..49497ae85 100644 --- a/features/options/prepare/help.feature +++ b/features/options/prepare/help.feature @@ -3,8 +3,7 @@ Feature: osrm-prepare command line options: help Background: Given the profile "testbot" - - @todo + Scenario: osrm-prepare - Help should be shown when no options are passed When I run "osrm-prepare" Then stderr should be empty @@ -48,4 +47,4 @@ Feature: osrm-prepare command line options: help And stdout should contain "--profile" And stdout should contain "--threads" And stdout should contain 15 lines - And it should exit with code 0 \ No newline at end of file + And it should exit with code 0 diff --git a/features/options/prepare/invalid.feature b/features/options/prepare/invalid.feature index a17606613..9eb4b757a 100644 --- a/features/options/prepare/invalid.feature +++ b/features/options/prepare/invalid.feature @@ -3,11 +3,10 @@ Feature: osrm-prepare command line options: invalid options Background: Given the profile "testbot" - - @todo + Scenario: osrm-prepare - Non-existing option When I run "osrm-prepare --fly-me-to-the-moon" Then stdout should be empty - And stderr should contain "exception" + And stderr should contain "Exception" And stderr should contain "fly-me-to-the-moon" - And it should exit with code 1 \ No newline at end of file + And it should exit with code 1 diff --git a/prepare.cpp b/prepare.cpp index 5664a5035..145309e97 100644 --- a/prepare.cpp +++ b/prepare.cpp @@ -121,34 +121,34 @@ int main (int argc, char *argv[]) { } if(option_variables.count("help")) { - SimpleLogger().Write() << std::endl << visible_options; + SimpleLogger().Write() << "\n" << visible_options; return 0; } boost::program_options::notify(option_variables); - // if(boost::filesystem::is_regular_file(config_file_path)) { - // SimpleLogger().Write() << "Reading options from: " << config_file_path.c_str(); - // std::string config_str; - // PrepareConfigFile( config_file_path.c_str(), config_str ); - // std::stringstream config_stream( config_str ); - // boost::program_options::store(parse_config_file(config_stream, config_file_options), option_variables); - // boost::program_options::notify(option_variables); - // } - if(!option_variables.count("restrictions")) { - restrictions_path = std::string( input_path.c_str()) + ".restrictions"; + restrictions_path = std::string(input_path.string() + ".restrictions"); } if(!option_variables.count("input")) { - SimpleLogger().Write(logWARNING) << "No input file specified"; - SimpleLogger().Write() << visible_options; - return -1; + SimpleLogger().Write() << "\n" << visible_options; + return 0; + } + + if(!boost::filesystem::is_regular_file(input_path)) { + SimpleLogger().Write(logWARNING) << "Input file " << input_path.string() << " not found!"; + return 1; + } + + if(!boost::filesystem::is_regular_file(profile_path)) { + SimpleLogger().Write(logWARNING) << "Profile " << profile_path.string() << " not found!"; + return 1; } if(1 > requested_num_threads) { SimpleLogger().Write(logWARNING) << "Number of threads must be 1 or larger"; - return -1; + return 1; } SimpleLogger().Write() << "Input file: " << input_path.filename().string(); @@ -183,11 +183,11 @@ int main (int argc, char *argv[]) { std::ifstream in; in.open (input_path.c_str(), std::ifstream::in | std::ifstream::binary); - std::string nodeOut(input_path.c_str()); nodeOut += ".nodes"; - std::string edgeOut(input_path.c_str()); edgeOut += ".edges"; - std::string graphOut(input_path.c_str()); graphOut += ".hsgr"; - std::string rtree_nodes_path(input_path.c_str()); rtree_nodes_path += ".ramIndex"; - std::string rtree_leafs_path(input_path.c_str()); rtree_leafs_path += ".fileIndex"; + std::string nodeOut(input_path.string() + ".nodes"); + std::string edgeOut(input_path.string() + ".edges"); + std::string graphOut(input_path.string() + ".hsgr"); + std::string rtree_nodes_path(input_path.string() + ".ramIndex"); + std::string rtree_leafs_path(input_path.string() + ".fileIndex"); /*** Setup Scripting Environment ***/ @@ -218,7 +218,7 @@ int main (int argc, char *argv[]) { lua_tostring(myLuaState,-1) << " occured in scripting block" << std::endl; - return -1; + return 1; } speedProfile.trafficSignalPenalty = 10*lua_tointeger(myLuaState, -1); @@ -227,7 +227,7 @@ int main (int argc, char *argv[]) { lua_tostring(myLuaState,-1) << " occured in scripting block" << std::endl; - return -1; + return 1; } speedProfile.uTurnPenalty = 10*lua_tointeger(myLuaState, -1); @@ -239,7 +239,7 @@ int main (int argc, char *argv[]) { if( edgeList.empty() ) { SimpleLogger().Write(logWARNING) << "The input data is empty, exiting."; - return -1; + return 1; } SimpleLogger().Write() << @@ -395,7 +395,7 @@ int main (int argc, char *argv[]) { SimpleLogger().Write(logWARNING) << "Failed at edges of node " << node << " of " << numberOfNodes; - return -1; + return 1; } //Serialize edges hsgr_output_stream.write((char*) ¤tEdge, sizeof(StaticGraph::_StrEdge)); @@ -419,14 +419,14 @@ int main (int argc, char *argv[]) { SimpleLogger().Write() << "finished preprocessing"; } catch(boost::program_options::too_many_positional_options_error& e) { SimpleLogger().Write(logWARNING) << "Only one file can be specified"; - return -1; + return 1; } catch(boost::program_options::error& e) { SimpleLogger().Write(logWARNING) << e.what(); - return -1; + return 1; } catch ( const std::exception &e ) { SimpleLogger().Write(logWARNING) << "Exception occured: " << e.what() << std::endl; - return -1; + return 1; } return 0; } From bf27f41f52f91b7f63a62d3a08096bd88fe4cc96 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Mon, 21 Apr 2014 17:21:02 +0200 Subject: [PATCH 26/27] implementing option tests marked @todo --- features/options/prepare/files.feature | 4 ---- 1 file changed, 4 deletions(-) diff --git a/features/options/prepare/files.feature b/features/options/prepare/files.feature index fb078d0f1..de356a82b 100644 --- a/features/options/prepare/files.feature +++ b/features/options/prepare/files.feature @@ -17,11 +17,7 @@ Feature: osrm-prepare command line options: files When I run "osrm-prepare {base}.osrm --profile {profile}" Then stderr should be empty And it should exit with code 0 -<<<<<<< Updated upstream - -======= ->>>>>>> Stashed changes Scenario: osrm-prepare - Order of options should not matter When I run "osrm-prepare --profile {profile} {base}.osrm" Then stderr should be empty From 2ea45c0c58bb2a217dda4614cf2efb621b379f66 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Mon, 21 Apr 2014 17:29:48 +0200 Subject: [PATCH 27/27] implement tests marked todo, implements #986 --- features/testbot/bad.feature | 2 +- features/testbot/protobuffer.feature | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/features/testbot/bad.feature b/features/testbot/bad.feature index f28b07295..f0f3cfacc 100644 --- a/features/testbot/bad.feature +++ b/features/testbot/bad.feature @@ -12,7 +12,7 @@ Feature: Handle bad data in a graceful manner | nodes | When the data has been prepared - Then "osrm-extract" should return code 255 + Then "osrm-extract" should return code 1 Scenario: Only dead-end oneways Given the node map diff --git a/features/testbot/protobuffer.feature b/features/testbot/protobuffer.feature index 7d8329ad9..a77ff34a6 100644 --- a/features/testbot/protobuffer.feature +++ b/features/testbot/protobuffer.feature @@ -8,7 +8,7 @@ Feature: Importing protobuffer (.pbf) format Background: Given the profile "testbot" And the import format "pbf" - + Scenario: Testbot - Protobuffer import, nodes and ways Given the node map | | | | d | @@ -88,7 +88,7 @@ Feature: Importing protobuffer (.pbf) format | nodes | When the data has been prepared - Then "osrm-extract" should return code 255 + Then "osrm-extract" should return code 1 Scenario: Testbot - Protobuffer import, streetnames with UTF characters