# # # patch "ChangeLog" # from [c987ea8784a09997591ff5625b42c39886dace67] # to [84a29f98ca8c2f76e1e2a0b7c7ae50be47eef5d6] # # patch "database.cc" # from [e72a2c3e9de3c9d3e1ece2ecd9f6939f18ee8ed7] # to [688eaa61e7485a72cc4593c8f90f6d0b5cf6f832] # # patch "database.hh" # from [9713f6e00494fbf2ca1a4b3bdff0d3cbe9b7a545] # to [90abb4ec87c1b090257a07d79be1c3e33d931a6a] # # patch "monotone.cc" # from [b317b8de7b802a444e37ef76aa1f8dad46e9852e] # to [eca87b7afbcce22d039be4b46b43240069177c26] # # patch "schema_migration.cc" # from [04a62f69aacaa7b4bbce7a3eb8759895c68b9205] # to [642fd28cf4c1e900dd1d747cbe1c8d1acec7ca02] # # patch "schema_migration.hh" # from [62bbd4fe56ec69ab000f76a88f3cf4219cc88034] # to [7d49cb153197c91b0572b76cdc72748cadfda57f] # # patch "tests/dump_on_crash/__driver__.lua" # from [f9fac4cb9adcc4861a5420fc3efdd975939ad6b9] # to [318446527188e10853add4d6c9b7b82f450f34c8] # ============================================================ --- ChangeLog c987ea8784a09997591ff5625b42c39886dace67 +++ ChangeLog 84a29f98ca8c2f76e1e2a0b7c7ae50be47eef5d6 @@ -1,5 +1,33 @@ 2007-01-20 Zack Weinberg + * database.cc (sql_contexts): Delete write-only variable and all + code that touches it. + (database::check_format): Fix grammar error. + (check_sqlite_format_version): Delete. + (database::sql): Restructure with a single N-way mode argument. + Move logic for creating new databases ... + (database::initialize): ... here. Use open() directly. + (database::ensure_open_for_maintenance) + (database::check_db_nonexistent): New functions. + (database::dump, database::load, database::version, database::migrate) + (database::test_migration_step): Use new functions as + appropriate. Don't call close(). + (database::info): Similarly. Do a dummy query first thing to force + sqlite to look at the file and tell us if it's a database or not. + (database::close): Collapse into database destructor. + (database::open): Use assert_sqlite3_ok instead of hand-rolled + diagnostic; special case SQLITE_NOMEM. + (assert_sqlite3_ok): Move to ... + * schema_migration.cc: ... here; merge with 'error'. Adjust users + of 'error' to match. Add aux message for SQLITE_{CORRUPT,NOTADB}. + Don't give aux message for SQLITE_ERROR anymore. Special case + SQLITE_NOMEM. + * schema_migration.hh, database.hh: Update declarations. + + * monotone.cc (cpp_main): Report bad_alloc as what it is: memory + exhaustion, not a bug. + * tests/dump_on_crash: Adjust to match. + * database.cc: Make database::info work on arbitrarily screwed up databases. Also make its output consistently formatted and easier to read. Check for SQLite 3 database in more places. ============================================================ --- database.cc e72a2c3e9de3c9d3e1ece2ecd9f6939f18ee8ed7 +++ database.cc 688eaa61e7485a72cc4593c8f90f6d0b5cf6f832 @@ -101,9 +101,6 @@ namespace }; return q; } - - // track all open databases for close_all_databases() handler - set sql_contexts; } struct query @@ -199,7 +196,7 @@ database::check_format() if (have_revisions && (!have_rosters || !have_heights)) // must be an upgrade that requires rosters be regenerated E(false, - F("database %s misses some cached data\n" + F("database %s lacks some cached data\n" "run '%s db regenerate_caches' to restore use of this database") % filename % ui.prog_name); else @@ -229,120 +226,44 @@ database::set_app(app_state * app) __app = app; } -static void -check_sqlite_format_version(system_path const & filename) -{ - // sqlite 3 files begin with this constant string - // (version 2 files begin with a different one) - string version_string("SQLite format 3"); - - ifstream file(filename.as_external().c_str()); - N(file, F("unable to probe database version in file %s") % filename); - - for (string::const_iterator i = version_string.begin(); - i != version_string.end(); ++i) - { - char c; - file.get(c); - N(c == *i, F("database %s is not an sqlite version 3 file, " - "try dump and reload") % filename); - } -} - - -static void -assert_sqlite3_ok(sqlite3 *s) -{ - int errcode = sqlite3_errcode(s); - - if (errcode == SQLITE_OK) return; - - const char * errmsg = sqlite3_errmsg(s); - - // first log the code so we can find _out_ what the confusing code - // was... note that code does not uniquely identify the errmsg, unlike - // errno's. - L(FL("sqlite error: %d: %s") % errcode % errmsg); - - // sometimes sqlite is not very helpful - // so we keep a table of errors people have gotten and more helpful versions - // note: if you update this, try to keep the similar function in - // schema_migration.cc consistent. - const char * auxiliary_message = ""; - switch (errcode) - { - case SQLITE_ERROR: - case SQLITE_IOERR: - case SQLITE_CANTOPEN: - case SQLITE_PROTOCOL: - auxiliary_message - = _("make sure database and containing directory are writeable\n" - "and you have not run out of disk space"); - break; - default: - break; - } - // if the last message is empty, the \n will be stripped off too - E(false, F("sqlite error: %s\n%s") % errmsg % auxiliary_message); -} - struct sqlite3 * -database::sql(bool init, bool migrating_format) +database::sql(enum open_mode mode) { if (! __sql) { check_filename(); + check_db_exists(); + open(); - if (! init) + if (mode != schema_bypass_mode) { - check_db_exists(); - check_sqlite_format_version(filename); - } + check_sql_schema(__sql, filename); + install_functions(__app); - open(); - - if (init) - { - sqlite3_exec(__sql, schema_constant, NULL, NULL, NULL); - assert_sqlite3_ok(__sql); + if (mode != format_bypass_mode) + check_format(); } - - check_sql_schema(__sql, filename); - install_functions(__app); - - if (!migrating_format) - check_format(); } else - { - I(!init); - I(!migrating_format); - } + I(mode == normal_mode); + return __sql; } void database::initialize() { - if (__sql) - throw oops("cannot initialize database while it is open"); + check_filename(); + check_db_nonexistent(); + open(); - require_path_is_nonexistent(filename, - F("could not initialize database: %s: already exists") - % filename); + sqlite3_exec(__sql, schema_constant, NULL, NULL, NULL); + assert_sqlite3_ok(__sql); - system_path journal(filename.as_internal() + "-journal"); - require_path_is_nonexistent(journal, - F("existing (possibly stale) journal file '%s' " - "has same stem as new database '%s'\n" - "cancelling database creation") - % journal % filename); - - sqlite3 *s = sql(true); - I(s != NULL); + // make sure what we wanted is what we got + check_sql_schema(__sql, filename); } - struct dump_request { @@ -444,11 +365,8 @@ database::dump(ostream & out) void database::dump(ostream & out) { - // don't care about schema checking etc. - check_filename(); - check_db_exists(); - check_sqlite_format_version(filename); - open(); + ensure_open_for_maintenance(); + { transaction_guard guard(*this); dump_request req; @@ -472,7 +390,6 @@ database::dump(ostream & out) out << "COMMIT;\n"; guard.commit(); } - close(); } void @@ -482,10 +399,7 @@ database::load(istream & in) string sql_stmt; check_filename(); - - require_path_is_nonexistent(filename, - F("cannot create %s; it already exists") % filename); - + check_db_nonexistent(); open(); // the page size can only be set before any other commands have been executed @@ -559,17 +473,21 @@ format_sqlite_error_for_info(informative err.append("]"); return err; } - void database::info(ostream & out) { // don't check the schema - check_filename(); - check_db_exists(); - check_sqlite_format_version(filename); - open(); - + ensure_open_for_maintenance(); + + // do a dummy query to confirm that the database file is an sqlite3 + // database. (this doesn't happen on open() because sqlite postpones the + // actual file open until the first access. we can't piggyback this on + // any of the real queries because they all trap errors in case tables are + // missing.) + sqlite3_exec(__sql, "SELECT 1 FROM sqlite_master LIMIT 0", 0, 0, 0); + assert_sqlite3_ok(__sql); + vector counts; counts.push_back(count("rosters")); counts.push_back(count("roster_deltas")); @@ -677,64 +595,48 @@ database::info(ostream & out) form = form % cache_size(); out << form.str() << "\n"; // final newline is kept out of the translation - - close(); } void database::version(ostream & out) { - check_filename(); - check_db_exists(); - check_sqlite_format_version(filename); - open(); - + ensure_open_for_maintenance(); out << (F("database schema version: %s") % describe_sql_schema(__sql)).str() << "\n"; - - close(); } void database::migrate() { - check_filename(); - check_db_exists(); - check_sqlite_format_version(filename); - open(); - + ensure_open_for_maintenance(); migrate_sql_schema(__sql, *__app); - - close(); } void database::test_migration_step(string const & schema) { - check_filename(); - check_db_exists(); - check_sqlite_format_version(filename); - open(); - + ensure_open_for_maintenance(); ::test_migration_step(__sql, *__app, schema); - - close(); } void database::ensure_open() { - sqlite3 *s = sql(); - I(s != NULL); + sql(); } void database::ensure_open_for_format_changes() { - sqlite3 *s = sql(false, true); - I(s != NULL); + sql(format_bypass_mode); } +void +database::ensure_open_for_maintenance() +{ + sql(schema_bypass_mode); +} + database::~database() { L(FL("statement cache statistics")); @@ -746,7 +648,11 @@ database::~database() // trigger destructors to finalize cached statements statement_cache.clear(); - close(); + if (__sql) + { + sqlite3_close(__sql); + __sql = 0; + } } void @@ -3159,6 +3065,22 @@ database::check_db_exists() F("%s is a directory, not a database") % filename); } +void +database::check_db_nonexistent() +{ + require_path_is_nonexistent(filename, + F("database %s already exists") + % filename); + + system_path journal(filename.as_internal() + "-journal"); + require_path_is_nonexistent(journal, + F("existing (possibly stale) journal file '%s' " + "has same stem as new database '%s'\n" + "cancelling database creation") + % journal % filename); + +} + bool database::database_specified() { @@ -3169,35 +3091,15 @@ database::open() void database::open() { - int error; - I(!__sql); - error = sqlite3_open(filename.as_external().c_str(), &__sql); + if (sqlite3_open(filename.as_external().c_str(), &__sql) == SQLITE_NOMEM) + throw std::bad_alloc(); - if (__sql) - { - I(sql_contexts.find(__sql) == sql_contexts.end()); - sql_contexts.insert(__sql); - } - - N(!error, (F("could not open database '%s': %s") - % filename % string(sqlite3_errmsg(__sql)))); + I(__sql); + assert_sqlite3_ok(__sql); } -void -database::close() -{ - if (__sql) - { - sqlite3_close(__sql); - I(sql_contexts.find(__sql) != sql_contexts.end()); - sql_contexts.erase(__sql); - __sql = 0; - } -} - - // transaction guards transaction_guard::transaction_guard(database & d, bool exclusive, ============================================================ --- database.hh 9713f6e00494fbf2ca1a4b3bdff0d3cbe9b7a545 +++ database.hh 90abb4ec87c1b090257a07d79be1c3e33d931a6a @@ -90,14 +90,17 @@ private: app_state * __app; struct sqlite3 * __sql; + enum open_mode { normal_mode = 0, + schema_bypass_mode, + format_bypass_mode }; + void install_functions(app_state * app); - struct sqlite3 * sql(bool init = false, bool migrating_format = false); + struct sqlite3 * sql(enum open_mode mode = normal_mode); void check_filename(); void check_db_exists(); + void check_db_nonexistent(); void open(); - void close(); - void check_format(); public: @@ -111,6 +114,9 @@ public: bool is_dbfile(any_path const & file); void ensure_open(); void ensure_open_for_format_changes(); +private: + void ensure_open_for_maintenance(); +public: void check_is_not_rosterified(); bool database_specified(); ============================================================ --- monotone.cc b317b8de7b802a444e37ef76aa1f8dad46e9852e +++ monotone.cc eca87b7afbcce22d039be4b46b43240069177c26 @@ -302,6 +302,11 @@ cpp_main(int argc, char ** argv) // an error has already been printed return 1; } + catch (std::bad_alloc) + { + ui.inform(_("error: memory exhausted")); + return 1; + } catch (std::exception const & ex) { ui.fatal_exception (ex); ============================================================ --- schema_migration.cc 04a62f69aacaa7b4bbce7a3eb8759895c68b9205 +++ schema_migration.cc 642fd28cf4c1e900dd1d747cbe1c8d1acec7ca02 @@ -33,6 +33,70 @@ // Wrappers around the bare sqlite3 API. We do not use sqlite3_exec because // we want the better error handling that sqlite3_prepare_v2 gives us. +void +assert_sqlite3_ok(sqlite3 * db) +{ + int errcode = sqlite3_errcode(db); + + if (errcode == SQLITE_OK) + return; + + char const * errmsg = sqlite3_errmsg(db); + + // first log the code so we can find _out_ what the confusing code + // was... note that code does not uniquely identify the errmsg, unlike + // errno's. + L(FL("sqlite error: %d: %s") % errcode % errmsg); + + // Check the string to see if it looks like an informative_failure + // thrown from within an SQL extension function, caught, and turned + // into a call to sqlite3_result_error. (Extension functions have to + // do this to avoid corrupting sqlite's internal state.) If it is, + // rethrow it rather than feeding it to E(), lest we get "error: + // sqlite error: error: " ugliness. + char const *pfx = _("error: "); + if (!std::strncmp(errmsg, pfx, strlen(pfx))) + throw informative_failure(errmsg); + + // sometimes sqlite is not very helpful + // so we keep a table of errors people have gotten and more helpful versions + char const * auxiliary_message = ""; + switch (errcode) + { + // All memory-exhaustion conditions should give the same diagnostic. + case SQLITE_NOMEM: + throw std::bad_alloc(); + + // These diagnostics generally indicate an operating-system-level + // failure. It would be nice to throw strerror(errno) in there but + // we cannot assume errno is still valid by the time we get here. + case SQLITE_IOERR: + case SQLITE_CANTOPEN: + case SQLITE_PROTOCOL: + auxiliary_message + = _("make sure database and containing directory are writeable\n" + "and you have not run out of disk space"); + break; + + // These error codes may indicate someone is trying to load a database + // so old that it's in sqlite 2's disk format (monotone 0.16 or + // older). + case SQLITE_CORRUPT: + case SQLITE_NOTADB: + auxiliary_message + = _("(if this is a database last used by monotone 0.16 or older,\n" + "you must follow a special procedure to make it usable again.\n" + "see the file UPGRADE, in the distribution, for instructions.)"); + + default: + break; + } + + // if the auxiliary message is empty, the \n will be stripped off too + E(false, F("sqlite error: %s\n%s") % errmsg % auxiliary_message); +} + + namespace { struct sql @@ -45,8 +109,8 @@ namespace char const * after; L(FL("executing SQL '%s'") % cmd); - if (sqlite3_prepare_v2(db, cmd, strlen(cmd), &s, &after)) - error(db); + sqlite3_prepare_v2(db, cmd, strlen(cmd), &s, &after); + assert_sqlite3_ok(db); I(s); if (afterp) @@ -78,7 +142,8 @@ namespace sqlite3 * db = sqlite3_db_handle(stmt); sqlite3_finalize(stmt); stmt = 0; - error(db); + assert_sqlite3_ok(db); + I(false); } int column_int(int col) { @@ -126,52 +191,13 @@ namespace void (*fn)(sqlite3_context *, int, sqlite3_value **)) { - if (sqlite3_create_function(db, name, -1, SQLITE_UTF8, 0, fn, 0, 0)) - error(db); + sqlite3_create_function(db, name, -1, SQLITE_UTF8, 0, fn, 0, 0); + assert_sqlite3_ok(db); } private: sqlite3_stmt * stmt; int ncols; - - static void NORETURN - error(sqlite3 * db) - { - // note: useful error messages should be kept consistent with - // assert_sqlite3_ok() in database.cc - char const * errmsg = sqlite3_errmsg(db); - int errcode = sqlite3_errcode(db); - - L(FL("sqlite error: %d: %s") % errcode % errmsg); - - // Check the string to see if it looks like an informative_failure - // thrown from within an SQL extension function, caught, and turned - // into a call to sqlite3_result_error. (Extension functions have to - // do this to avoid corrupting sqlite's internal state.) If it is, - // rethrow it rather than feeding it to E(), lest we get "error: - // sqlite error: error: " ugliness. - char const *pfx = _("error: "); - if (!std::strncmp(errmsg, pfx, strlen(pfx))) - throw informative_failure(errmsg); - - char const * auxiliary_message = ""; - switch (errcode) - { - case SQLITE_ERROR: // ??? take this out - 3.3.9 seems to generate - // it mostly for logic errors in the SQL, - // not environmental problems - case SQLITE_IOERR: - case SQLITE_CANTOPEN: - case SQLITE_PROTOCOL: - auxiliary_message - = _("make sure database and containing directory are writeable\n" - "and you have not run out of disk space"); - break; - default: break; - } - - E(false, F("sqlite error: %s\n%s") % errmsg % auxiliary_message); - } }; struct transaction ============================================================ --- schema_migration.hh 62bbd4fe56ec69ab000f76a88f3cf4219cc88034 +++ schema_migration.hh 7d49cb153197c91b0572b76cdc72748cadfda57f @@ -27,6 +27,10 @@ std::string describe_sql_schema(sqlite3 void check_sql_schema(sqlite3 * db, system_path const & filename); std::string describe_sql_schema(sqlite3 * db); +// utility routine shared with database.cc +void assert_sqlite3_ok(sqlite3 * db); + +// debugging void test_migration_step(sqlite3 * db, app_state & app, std::string const & schema); ============================================================ --- tests/dump_on_crash/__driver__.lua f9fac4cb9adcc4861a5420fc3efdd975939ad6b9 +++ tests/dump_on_crash/__driver__.lua 318446527188e10853add4d6c9b7b82f450f34c8 @@ -16,8 +16,7 @@ check(exists("fork")) check(exists("fork")) -- all the exceptions caught in monotone.cc and translated to error messages -for _,tag in pairs({ 'std::bad_alloc', - 'std::bad_cast', +for _,tag in pairs({ 'std::bad_cast', 'std::bad_typeid', 'std::bad_exception', 'std::domain_error', @@ -35,6 +34,11 @@ end check(exists("fork")) end +-- bad_alloc is special +remove("fork") +check(mtn("crash", "std::bad_alloc", "--dump=fork"), 1, false, false) +check(not exists("fork")) + -- selected signals - note hardwired signal numbers :( skip_if(ostype == "Windows") remove("fork")