diff options
| -rw-r--r-- | sql/log_event.cc | 32 | ||||
| -rw-r--r-- | sql/slave.cc | 18 | ||||
| -rw-r--r-- | sql/slave.h | 1 | ||||
| -rw-r--r-- | sql/sql_class.h | 24 | ||||
| -rw-r--r-- | sql/sql_db.cc | 159 |
5 files changed, 112 insertions, 122 deletions
diff --git a/sql/log_event.cc b/sql/log_event.cc index b4707826205..cf5dbb1e77c 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -1635,14 +1635,33 @@ void Query_log_event::print(FILE* file, PRINT_EVENT_INFO* print_event_info) */ #if defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT) + +static const char *rewrite_db(const char *db) +{ + if (replicate_rewrite_db.is_empty() || db == NULL) + return db; + I_List_iterator<i_string_pair> it(replicate_rewrite_db); + i_string_pair* tmp; + + while ((tmp=it++)) + { + if (strcmp(tmp->key, db) == 0) + return tmp->val; + } + return db; +} + + int Query_log_event::exec_event(struct st_relay_log_info* rli) { return exec_event(rli, query, q_len); } -int Query_log_event::exec_event(struct st_relay_log_info* rli, const char *query_arg, uint32 q_len_arg) +int Query_log_event::exec_event(struct st_relay_log_info* rli, + const char *query_arg, uint32 q_len_arg) { + const char *new_db= rewrite_db(db); int expected_error,actual_error= 0; /* Colleagues: please never free(thd->catalog) in MySQL. This would lead to @@ -1651,8 +1670,7 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli, const char *query Thank you. */ thd->catalog= catalog_len ? (char *) catalog : (char *)""; - thd->db_length= db_len; - thd->db= (char*) rewrite_db(db, &thd->db_length); + thd->set_db(new_db, strlen(new_db)); /* allocates a copy of 'db' */ thd->variables.auto_increment_increment= auto_increment_increment; thd->variables.auto_increment_offset= auto_increment_offset; @@ -1869,7 +1887,7 @@ end: TABLE uses the db.table syntax. */ thd->catalog= 0; - thd->reset_db(NULL, 0); // prevent db from being freed + thd->set_db(NULL, 0); /* will free the current database */ thd->query= 0; // just to be sure thd->query_length= 0; VOID(pthread_mutex_unlock(&LOCK_thread_count)); @@ -2817,8 +2835,8 @@ void Load_log_event::set_fields(const char* affected_db, int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, bool use_rli_only_for_errors) { - thd->db_length= db_len; - thd->db= (char*) rewrite_db(db, &thd->db_length); + const char *new_db= rewrite_db(db); + thd->set_db(new_db, strlen(new_db)); DBUG_ASSERT(thd->query == 0); thd->query_length= 0; // Should not be needed thd->query_error= 0; @@ -3018,7 +3036,7 @@ error: const char *remember_db= thd->db; VOID(pthread_mutex_lock(&LOCK_thread_count)); thd->catalog= 0; - thd->reset_db(NULL, 0); + thd->set_db(NULL, 0); /* will free the current database */ thd->query= 0; thd->query_length= 0; VOID(pthread_mutex_unlock(&LOCK_thread_count)); diff --git a/sql/slave.cc b/sql/slave.cc index 4da447c4bc3..b284f4a6a16 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -1177,24 +1177,6 @@ bool net_request_file(NET* net, const char* fname) } -const char *rewrite_db(const char* db, uint *new_len) -{ - if (replicate_rewrite_db.is_empty() || !db) - return db; - I_List_iterator<i_string_pair> it(replicate_rewrite_db); - i_string_pair* tmp; - - while ((tmp=it++)) - { - if (!strcmp(tmp->key, db)) - { - *new_len= (uint32)strlen(tmp->val); - return tmp->val; - } - } - return db; -} - /* From other comments and tests in code, it looks like sometimes Query_log_event and Load_log_event can have db == 0 diff --git a/sql/slave.h b/sql/slave.h index 7f08105c0b9..c355f7172a9 100644 --- a/sql/slave.h +++ b/sql/slave.h @@ -550,7 +550,6 @@ int add_table_rule(HASH* h, const char* table_spec); int add_wild_table_rule(DYNAMIC_ARRAY* a, const char* table_spec); void init_table_rule_hash(HASH* h, bool* h_inited); void init_table_rule_array(DYNAMIC_ARRAY* a, bool* a_inited); -const char *rewrite_db(const char* db, uint *new_db_len); const char *print_slave_db_safe(const char *db); int check_expected_error(THD* thd, RELAY_LOG_INFO* rli, int error_code); void skip_load_data_infile(NET* net); diff --git a/sql/sql_class.h b/sql/sql_class.h index 1ba104df2a4..eb075dd54bb 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1574,21 +1574,23 @@ public: /* Initialize the current database from a NULL-terminated string with length + If we run out of memory, we free the current database and return TRUE. + This way the user will notice the error as there will be no current + database selected (in addition to the error message set by malloc). */ - void set_db(const char *new_db, uint new_db_len) + bool set_db(const char *new_db, uint new_db_len) { - if (new_db) + /* Do not reallocate memory if current chunk is big enough. */ + if (db && new_db && db_length >= new_db_len) + memcpy(db, new_db, new_db_len+1); + else { - /* Do not reallocate memory if current chunk is big enough. */ - if (db && db_length >= new_db_len) - memcpy(db, new_db, new_db_len+1); - else - { - safeFree(db); - db= my_strdup_with_length(new_db, new_db_len, MYF(MY_WME)); - } - db_length= db ? new_db_len: 0; + x_free(db); + db= new_db ? my_strdup_with_length(new_db, new_db_len, MYF(MY_WME)) : + NULL; } + db_length= db ? new_db_len : 0; + return new_db && !db; } void reset_db(char *new_db, uint new_db_len) { diff --git a/sql/sql_db.cc b/sql/sql_db.cc index 44947384b32..902539dfdec 100644 --- a/sql/sql_db.cc +++ b/sql/sql_db.cc @@ -781,32 +781,13 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) exit: (void)sp_drop_db_routines(thd, db); /* QQ Ignore errors for now */ /* - If this database was the client's selected database, we silently change the - client's selected database to nothing (to have an empty SELECT DATABASE() - in the future). For this we free() thd->db and set it to 0. But we don't do - free() for the slave thread. Indeed, doing a x_free() on it leads to nasty - problems (i.e. long painful debugging) because in this thread, thd->db is - the same as data_buf and db of the Query_log_event which is dropping the - database. So if you free() thd->db, you're freeing data_buf. You set - thd->db to 0 but not data_buf (thd->db and data_buf are two distinct - pointers which point to the same place). Then in ~Query_log_event(), we - have 'if (data_buf) free(data_buf)' data_buf is !=0 so this makes a - DOUBLE free(). - Side effects of this double free() are, randomly (depends on the machine), - when the slave is replicating a DROP DATABASE: - - garbage characters in the error message: - "Error 'Can't drop database 'test2'; database doesn't exist' on query - 'h4zI©'" - - segfault - - hang in "free(vio)" (yes!) in the I/O or SQL slave threads (so slave - server hangs at shutdown etc). + If this database was the client's selected database, we silently + change the client's selected database to nothing (to have an empty + SELECT DATABASE() in the future). For this we free() thd->db and set + it to 0. */ if (thd->db && !strcmp(thd->db, db)) - { - if (!(thd->slave_thread)) /* a slave thread will free it itself */ - x_free(thd->db); - thd->reset_db(NULL, 0); - } + thd->set_db(NULL, 0); VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); start_waiting_global_read_lock(thd); exit2: @@ -1099,38 +1080,52 @@ err: /* - Change default database. + Change the current database. SYNOPSIS mysql_change_db() - thd Thread handler - name Databasename - no_access_check True don't do access check. In this case name may be "" + thd thread handle + name database name + no_access_check if TRUE, don't do access check. In this + case name may be "" DESCRIPTION - Becasue the database name may have been given directly from the - communication packet (in case of 'connect' or 'COM_INIT_DB') - we have to do end space removal in this function. + Check that the database name corresponds to a valid and + existent database, check access rights (unless called with + no_access_check), and set the current database. This function + is called to change the current database upon user request + (COM_CHANGE_DB command) or temporarily, to execute a stored + routine. NOTES - Do as little as possible in this function, as it is not called for the - replication slave SQL thread (for that thread, setting of thd->db is done - in ::exec_event() methods of log_event.cc). - - This function does not send anything, including error messages to the - client, if that should be sent to the client, call net_send_error after - this function. + This function is not the only way to switch the database that + is currently employed. When the replication slave thread + switches the database before executing a query, it calls + thd->set_db directly. However, if the query, in turn, uses + a stored routine, the stored routine will use this function, + even if it's run on the slave. + + This function allocates the name of the database on the system + heap: this is necessary to be able to uniformly change the + database from any module of the server. Up to 5.0 different + modules were using different memory to store the name of the + database, and this led to memory corruption: a stack pointer + set by Stored Procedures was used by replication after the + stack address was long gone. + + This function does not send anything, including error + messages, to the client. If that should be sent to the client, + call net_send_error after this function. RETURN VALUES - 0 ok + 0 OK 1 error */ bool mysql_change_db(THD *thd, const char *name, bool no_access_check) { - int length, db_length; - char *dbname= thd->slave_thread ? (char *) name : - my_strdup((char *) name, MYF(MY_WME)); + int path_length, db_length; + char *db_name; char path[FN_REFLEN]; HA_CREATE_INFO create; bool system_db= 0; @@ -1142,32 +1137,35 @@ bool mysql_change_db(THD *thd, const char *name, bool no_access_check) DBUG_ENTER("mysql_change_db"); DBUG_PRINT("enter",("name: '%s'",name)); - LINT_INIT(db_length); - - /* dbname can only be NULL if malloc failed */ - if (!dbname || !(db_length= strlen(dbname))) + if (name == NULL || name[0] == '\0' && no_access_check == FALSE) { - if (no_access_check && dbname) - { - /* Called from SP when orignal database was not set */ - system_db= 1; - goto end; - } - if (!(thd->slave_thread)) - x_free(dbname); /* purecov: inspected */ - my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), - MYF(0)); /* purecov: inspected */ + my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); DBUG_RETURN(1); /* purecov: inspected */ } - if (check_db_name(dbname)) + else if (name[0] == '\0') { - my_error(ER_WRONG_DB_NAME, MYF(0), dbname); - if (!(thd->slave_thread)) - my_free(dbname, MYF(0)); + /* Called from SP to restore the original database, which was NULL */ + DBUG_ASSERT(no_access_check); + system_db= 1; + db_name= NULL; + db_length= 0; + goto end; + } + /* + Now we need to make a copy because check_db_name requires a + non-constant argument. TODO: fix check_db_name. + */ + if ((db_name= my_strdup(name, MYF(MY_WME))) == NULL) + DBUG_RETURN(1); /* the error is set */ + db_length= strlen(db_name); + if (check_db_name(db_name)) + { + my_error(ER_WRONG_DB_NAME, MYF(0), db_name); + my_free(db_name, MYF(0)); DBUG_RETURN(1); } - DBUG_PRINT("info",("Use database: %s", dbname)); - if (!my_strcasecmp(system_charset_info, dbname, information_schema_name.str)) + DBUG_PRINT("info",("Use database: %s", db_name)); + if (!my_strcasecmp(system_charset_info, db_name, information_schema_name.str)) { system_db= 1; #ifndef NO_EMBEDDED_ACCESS_CHECKS @@ -1182,45 +1180,36 @@ bool mysql_change_db(THD *thd, const char *name, bool no_access_check) if (test_all_bits(sctx->master_access, DB_ACLS)) db_access=DB_ACLS; else - db_access= (acl_get(sctx->host, sctx->ip, sctx->priv_user, dbname, 0) | + db_access= (acl_get(sctx->host, sctx->ip, sctx->priv_user, db_name, 0) | sctx->master_access); if (!(db_access & DB_ACLS) && (!grant_option || - check_grant_db(thd,dbname))) + check_grant_db(thd,db_name))) { my_error(ER_DBACCESS_DENIED_ERROR, MYF(0), sctx->priv_user, sctx->priv_host, - dbname); + db_name); mysql_log.write(thd, COM_INIT_DB, ER(ER_DBACCESS_DENIED_ERROR), - sctx->priv_user, sctx->priv_host, dbname); - if (!(thd->slave_thread)) - my_free(dbname,MYF(0)); + sctx->priv_user, sctx->priv_host, db_name); + my_free(db_name, MYF(0)); DBUG_RETURN(1); } } #endif - (void) sprintf(path,"%s/%s",mysql_data_home,dbname); - length=unpack_dirname(path,path); // Convert if not unix - if (length && path[length-1] == FN_LIBCHAR) - path[length-1]=0; // remove ending '\' + (void) sprintf(path,"%s/%s", mysql_data_home, db_name); + path_length= unpack_dirname(path, path); // Convert if not UNIX + if (path_length && path[path_length-1] == FN_LIBCHAR) + path[path_length-1]= '\0'; // remove ending '\' if (my_access(path,F_OK)) { - my_error(ER_BAD_DB_ERROR, MYF(0), dbname); - if (!(thd->slave_thread)) - my_free(dbname,MYF(0)); + my_error(ER_BAD_DB_ERROR, MYF(0), db_name); + my_free(db_name, MYF(0)); DBUG_RETURN(1); } end: - if (!(thd->slave_thread)) - x_free(thd->db); - if (dbname && dbname[0] == 0) - { - if (!(thd->slave_thread)) - my_free(dbname, MYF(0)); - thd->reset_db(NULL, 0); - } - else - thd->reset_db(dbname, db_length); // THD::~THD will free this + x_free(thd->db); + DBUG_ASSERT(db_name == NULL || db_name[0] != '\0'); + thd->reset_db(db_name, db_length); // THD::~THD will free this #ifndef NO_EMBEDDED_ACCESS_CHECKS if (!no_access_check) sctx->db_access= db_access; |