# # add_file "tests/t_database_check_minor.at" # # patch "ChangeLog" # from [1e1c391bdd1f85ec83bc7bf3cacc4680ba6ddbdd] # to [b66dd11efda1630f2785f0af8e3c9183683717da] # # patch "database_check.cc" # from [924b949941d72064a292451e72c28bfb2e80755c] # to [63ce24deafa543d9f38e21ae826d8230e3d4d44c] # # patch "netsync.cc" # from [b68476ffc5852e56195629f4998af91221e48dcd] # to [9c25bf74dc181c19938878d5928a9fbf7a4ce25e] # # patch "sanity.cc" # from [2fd252fa148582b35ba95b8ae368e2c500d4b01f] # to [87f3bbf0785ac602b26fc8f36ff4efdf475db220] # # patch "sanity.hh" # from [e7e2010f734f9e54499e14c700017b4f0577e9ac] # to [952408e83f22dc43392c7afcffc6e556cfd80d6b] # # patch "tests/t_database_check.at" # from [3ea731d55685488c6dadb5580b4cd04dc130e855] # to [b02fe408e53bcd4d7219183e218bc21c864dee82] # # patch "tests/t_database_check_minor.at" # from [] # to [eaba52f05e60be3f4bfc420af0722fd5ed63df02] # # patch "testsuite.at" # from [c9806d0931d6dec93838b7479f2d31b378be8453] # to [5c3edfba27e5b828d6080f873f035114c54e7823] # # patch "transforms.cc" # from [61593e946306f8bd5926c02fca2ea683e510d271] # to [ba125aa4b47dbf1e00df2af72ae5d1a40b7b0175] # --- ChangeLog +++ ChangeLog @@ -1,3 +1,16 @@ +2005-04-17 Nathaniel Smith + + * sanity.{hh,cc} (E, error_failure): New sort of invariant. + * netsync.cc (process_hello_cmd): Make initial pull message + more clear and friendly. + Also, if the key has changed, that is an error, not naughtiness. + * database_check.cc (check_db): Database problems are also errors, + not naughtiness. Revamp output in case of errors, to better + distinguish non-serious errors and serious errors. + * tests/t_database_check.at: Update accordingly. + * tests/t_database_check_minor.at: New test. + * testsuite.at: Add it. + 2005-04-17 Richard Levitte * transforms.cc (glob_to_regexp): New function that takes a glob --- database_check.cc +++ database_check.cc @@ -669,22 +669,28 @@ missing_certs + mismatched_certs + unchecked_sigs + bad_sigs + missing_keys; + // unreferenced files and manifests and mismatched certs are not actually + // serious errors; odd, but nothing will break. + size_t serious = missing_files + + missing_manifests + incomplete_manifests + + missing_revisions + incomplete_revisions + + mismatched_parents + mismatched_children + + bad_history + + missing_certs + + unchecked_sigs + bad_sigs + + missing_keys; - if (total > 0) - N(total == 0, - F("check complete: %d files; %d manifests; %d revisions; %d keys; %d certs; %d problems detected\n") - % checked_files.size() - % checked_manifests.size() - % checked_revisions.size() - % checked_keys.size() - % total_certs - % total); + P(F("check complete: %d files; %d manifests; %d revisions; %d keys; %d certs\n") + % checked_files.size() + % checked_manifests.size() + % checked_revisions.size() + % checked_keys.size() + % total_certs); + P(F("total problems detected: %d (%d serious)\n") % total % serious); + if (serious) + E(false, F("serious problems detected")); + else if (total) + P(F("minor problems detected\n")); else - P(F("check complete: %d files; %d manifests; %d revisions; %d keys; %d certs; database is good\n") - % checked_files.size() - % checked_manifests.size() - % checked_revisions.size() - % checked_keys.size() - % total_certs - ); + P(F("database is good\n")); } --- netsync.cc +++ netsync.cc @@ -1445,13 +1445,14 @@ P(F("I expected %s\n") % expected_key_hash); P(F("'monotone unset %s %s' overrides this check\n") % their_key_key.first % their_key_key.second); - N(false, F("server key changed")); + E(false, F("server key changed")); } } else { - W(F("first time connecting to server %s; authenticity can't be established\n") % peer_id); - W(F("their key is %s\n") % their_key_hash); + P(F("first time connecting to server %s\n") % peer_id); + P(F("I'll assume it's really them, but you might want to\n")); + P(F("double-check their key's fingerprint: %s\n") % their_key_hash); app.db.set_var(their_key_key, var_value(their_key_hash())); } if (!app.db.public_key_exists(their_key_hash)) --- sanity.cc +++ sanity.cc @@ -185,6 +185,17 @@ } void +sanity::error_failure(string const & expr, format const & explain, + string const & file, int line) +{ + string message; + log(format("%s:%d: detected error '%s' violated\n") % file % line % expr, + file.c_str(), line); + prefix_lines_with("error: ", explain.str(), message); + throw informative_failure(message); +} + +void sanity::invariant_failure(string const & expr, string const & file, int line) { --- sanity.hh +++ sanity.hh @@ -44,20 +44,22 @@ std::string filename; void log(boost::format const & fmt, - char const * file, int line); + char const * file, int line); void progress(boost::format const & fmt, - char const * file, int line); + char const * file, int line); void warning(boost::format const & fmt, - char const * file, int line); + char const * file, int line); void naughty_failure(std::string const & expr, boost::format const & explain, - std::string const & file, int line); + std::string const & file, int line); + void error_failure(std::string const & expr, boost::format const & explain, + std::string const & file, int line); void invariant_failure(std::string const & expr, - std::string const & file, int line); + std::string const & file, int line); void index_failure(std::string const & vec_expr, - std::string const & idx_expr, - unsigned long sz, - unsigned long idx, - std::string const & file, int line); + std::string const & idx_expr, + unsigned long sz, + unsigned long idx, + std::string const & file, int line); }; typedef std::runtime_error oops; @@ -107,7 +109,16 @@ } \ } while(0) +// E is for errors; they are normal (i.e., not a bug), but not necessarily +// attributable to user naughtiness +#define E(e, explain)\ +do { \ + if(UNLIKELY(!(e))) { \ + global_sanity.error_failure("E("#e")", (explain), __FILE__, __LINE__); \ + } \ +} while(0) + // we're interested in trapping index overflows early and precisely, // because they usually represent *very significant* logic errors. we use // an inline template function because the idx(...) needs to be used as an @@ -115,11 +126,11 @@ template inline T & checked_index(std::vector & v, - typename std::vector::size_type i, - char const * vec, - char const * index, - char const * file, - int line) + typename std::vector::size_type i, + char const * vec, + char const * index, + char const * file, + int line) { if (UNLIKELY(i >= v.size())) global_sanity.index_failure(vec, index, v.size(), i, file, line); @@ -128,11 +139,11 @@ template inline T const & checked_index(std::vector const & v, - typename std::vector::size_type i, - char const * vec, - char const * index, - char const * file, - int line) + typename std::vector::size_type i, + char const * vec, + char const * index, + char const * file, + int line) { if (UNLIKELY(i >= v.size())) global_sanity.index_failure(vec, index, v.size(), i, file, line); --- tests/t_database_check.at +++ tests/t_database_check.at @@ -66,7 +66,8 @@ AT_CHECK(grep '1 missing file' stderr, [0], [ignore], [ignore]) AT_CHECK(grep '2 incomplete manifest' stderr, [0], [ignore], [ignore]) AT_CHECK(grep '2 incomplete revision' stderr, [0], [ignore], [ignore]) -AT_CHECK(grep '5 problems' stderr, [0], [ignore], [ignore]) +AT_CHECK(grep 'total problems detected: 5' stderr, [0], [ignore], [ignore]) +AT_CHECK(grep '5 serious' stderr, [0], [ignore], [ignore]) # add an unreferenced file, and an unreferenced manifest that # references several files, none of which exist in the db --- tests/t_database_check_minor.at +++ tests/t_database_check_minor.at @@ -0,0 +1,40 @@ +AT_SETUP([db check and non-serious errors]) +MONOTONE_SETUP + +# Make sure that db check detects minor problems, but doesn't complain +# about them too loudly (and doesn't exit with error status). + +AT_DATA(fileX, [blah blah +]) +AT_DATA(fileY, [stuff stuff +]) +# manifestX is: +# +# e3f2b0427b57241225ba1ffc2b67fecd64d07613 fileX +# +# where e3f2 etc. is as above + +AT_DATA(manifestX, [@<:@mdata 8e5d7e5ffca2393cfca5625ac9c1a19a46489f92@:>@ +H4sIAAAAAAAAAEs1TjNKMjAxMk8yNTcyMTQyMk1KNExLSzZKMjNPS01OMTNJMTA3MzRWUEjL +zEmN4AIA90gN9zAAAAA= +@<:@end@:>@ +]) + +ADD_FILE(testfile, [more stuff +]) +COMMIT(testbranch) +REV=`BASE_REVISION` + +AT_CHECK(MONOTONE cert $REV author extra_author, [], [ignore], [ignore]) + +AT_CHECK(MONOTONE fload < fileX, [], [ignore], [ignore]) +AT_CHECK(MONOTONE fload < fileY, [], [ignore], [ignore]) +AT_CHECK(MONOTONE read < manifestX, [], [ignore], [ignore]) + +AT_CHECK(MONOTONE db check, [], [ignore], [stderr]) + +AT_CHECK(grep -q 'problems detected: 3' stderr) +AT_CHECK(grep -q '0 serious' stderr) +AT_CHECK(grep -q 'minor problems detected' stderr) + +AT_CLEANUP --- testsuite.at +++ testsuite.at @@ -563,3 +563,4 @@ m4_include(tests/t_update_nonexistent.at) m4_include(tests/t_override_author_date.at) m4_include(tests/t_add_stomp_file.at) +m4_include(tests/t_database_check_minor.at) --- transforms.cc +++ transforms.cc @@ -738,11 +738,6 @@ // - ] directly following an unescaped [ is escaped. string glob_to_regexp(const string & glob) { - struct bad_glob { - bad_glob() : what("Bad glob syntax") {} - string what; - }; - int in_braces = 0; // counter for levels if {} bool in_brackets = false; // flags if we're inside a [], which // has higher precedence than {}. @@ -817,8 +812,8 @@ tmp += '('; break; case '}': - if (in_braces == 0) - throw bad_glob(); + N(in_braces == 0, + F("trying to end a brace expression in a glob when none is started")); tmp += ')'; in_braces--; break; @@ -846,8 +841,10 @@ } } - if (in_braces != 0 || in_brackets) - throw bad_glob(); + N(in_brackets, + F("run-away bracket expression in glob")); + N(in_braces != 0, + F("run-away brace expression in glob")); #ifdef BUILD_UNIT_TESTS cerr << "DEBUG[glob_to_regexp]: output = \"" << tmp << "\"" << endl;