From 8f08c511ee781a99b63a74957ecb8547ac334f06 Mon Sep 17 00:00:00 2001 From: "monty@mashka.mysql.fi" <> Date: Thu, 7 Aug 2003 20:16:37 +0300 Subject: [PATCH] Review of changesets since last pull. (Mostly code cleanups) --- include/my_sys.h | 1 - myisam/mi_check.c | 2 + mysql-test/t/rpl_insert_id.test | 1 + mysys/mf_format.c | 10 +---- sql/log_event.cc | 79 +++++++++++++++++---------------- sql/mini_client.cc | 2 +- sql/mysqld.cc | 64 +++++++++++++++++--------- sql/slave.cc | 38 ++++++++-------- sql/slave.h | 4 +- sql/sql_acl.cc | 11 ++--- vio/viosslfactories.c | 2 +- 11 files changed, 114 insertions(+), 100 deletions(-) diff --git a/include/my_sys.h b/include/my_sys.h index acce118c7b8..7f8b8a80a1c 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -100,7 +100,6 @@ extern int NEAR my_errno; /* Last error in mysys */ #define MY_RETURN_REAL_PATH 32 /* return full path for file */ #define MY_SAFE_PATH 64 /* Return NULL if too long path */ #define MY_RELATIVE_PATH 128 /* name is relative to 'dir' */ -#define MY_QUOTE_SPACES 256 /* quote when the path has spaces */ /* My seek flags */ #define MY_SEEK_SET 0 diff --git a/myisam/mi_check.c b/myisam/mi_check.c index 4b28a88fc6e..0d7d7fae628 100644 --- a/myisam/mi_check.c +++ b/myisam/mi_check.c @@ -3563,11 +3563,13 @@ int update_state_info(MI_CHECK *param, MI_INFO *info,uint update) share->state.rec_per_key_rows=info->state->records; share->state.changed&= ~STATE_NOT_ANALYZED; if (info->state->records) + { for (i=0; istate.rec_per_key_part[i]=param->rec_per_key_part[i])) share->state.changed|= STATE_NOT_ANALYZED; } + } } if (update & (UPDATE_STAT | UPDATE_SORT | UPDATE_TIME | UPDATE_AUTO_INC)) { diff --git a/mysql-test/t/rpl_insert_id.test b/mysql-test/t/rpl_insert_id.test index d91bd02883e..49fefae72b8 100644 --- a/mysql-test/t/rpl_insert_id.test +++ b/mysql-test/t/rpl_insert_id.test @@ -4,6 +4,7 @@ # We also check how the foreign_key_check variable is replicated source include/master-slave.inc; +source include/have_innodb.inc connection master; drop table if exists t1; create table t1(a int auto_increment, key(a)); diff --git a/mysys/mf_format.c b/mysys/mf_format.c index 36d3908d310..937904847ee 100644 --- a/mysys/mf_format.c +++ b/mysys/mf_format.c @@ -111,15 +111,7 @@ my_string fn_format(my_string to, const char *name, const char *dir, strmov(buff,to); (void) my_readlink(to, buff, MYF(0)); } - if ( flag & MY_QUOTE_SPACES) - if ( strchr(to, ' ')) - { - char tmp_buff[FN_REFLEN]; - tmp_buff[0]='"'; - strxmov(tmp_buff+1,to,"\"",NullS); - strmov(to,tmp_buff); - } - DBUG_RETURN (to); + DBUG_RETURN(to); } /* fn_format */ diff --git a/sql/log_event.cc b/sql/log_event.cc index 96bcf0a2779..f7955be3b09 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -593,8 +593,9 @@ err: UNLOCK_MUTEX; if (error) { - sql_print_error("Error in Log_event::read_log_event(): '%s', \ -data_len=%d,event_type=%d",error,data_len,head[EVENT_TYPE_OFFSET]); + sql_print_error("\ +Error in Log_event::read_log_event(): '%s', data_len: %d, event_type: %d", + error,data_len,head[EVENT_TYPE_OFFSET]); my_free(buf, MYF(MY_ALLOW_ZERO_PTR)); /* The SQL slave thread will check if file->error<0 to know @@ -1433,8 +1434,8 @@ void Slave_log_event::print(FILE* file, bool short_form, char* last_db) return; print_header(file); fputc('\n', file); - fprintf(file, "Slave: master_host: '%s' master_port: %d \ -master_log: '%s' master_pos: %s\n", + fprintf(file, "\ +Slave: master_host: '%s' master_port: %d master_log: '%s' master_pos: %s\n", master_host, master_port, master_log, llstr(master_pos, llbuff)); } @@ -1755,7 +1756,7 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli) { int expected_error, actual_error= 0; init_sql_alloc(&thd->mem_root, 8192,0); - thd->db = rewrite_db((char*)db); + thd->db= (char*) rewrite_db(db); /* InnoDB internally stores the master log position it has processed so far; @@ -1811,15 +1812,16 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli) !ignored_error_code(expected_error)) { slave_print_error(rli, 0, - "Query '%s' did not get the same error as the query \ -got on master - got on master: '%s' (%d), got on slave: '%s' (%d) \ -(default database was '%s')", + "\ +Query '%s' caused different errors on master and slave. \ +Error on master: '%s' (%d), Error on slave: '%s' (%d). \ +Default database: '%s'", query, ER_SAFE(expected_error), expected_error, actual_error ? thd->net.last_error: "no error", actual_error, - print_slave_db_safe((char*)db)); + print_slave_db_safe(db)); thd->query_error= 1; } /* @@ -1837,11 +1839,12 @@ got on master - got on master: '%s' (%d), got on slave: '%s' (%d) \ */ else if (thd->query_error || thd->fatal_error) { - slave_print_error(rli,actual_error, "Error '%s' on query '%s' \ -(default database was '%s')", - actual_error ? thd->net.last_error : - "unexpected success or fatal error", query, - print_slave_db_safe((char*)db)); + slave_print_error(rli,actual_error, + "Error '%s' on query '%s'. Default database: '%s'", + (actual_error ? thd->net.last_error : + "unexpected success or fatal error"), + query, + print_slave_db_safe(db)); thd->query_error= 1; } } @@ -1896,7 +1899,7 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, bool use_rli_only_for_errors) { init_sql_alloc(&thd->mem_root, 8192,0); - thd->db = rewrite_db((char*)db); + thd->db= (char*) rewrite_db(db); DBUG_ASSERT(thd->query == 0); thd->query = 0; // Should not be needed thd->query_error = 0; @@ -1978,17 +1981,15 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, if (mysql_load(thd, &ex, &tables, field_list, handle_dup, net != 0, TL_WRITE)) thd->query_error = 1; + /* log_pos is the position of the LOAD event in the master log */ if (thd->cuted_fields) - /* - log_pos is the position of the LOAD - event in the master log - */ - sql_print_error("Slave: load data infile at position %s in log \ -'%s' produced %d warning(s) (loaded table was '%s', database was '%s')", + sql_print_error("\ +Slave: load data infile on table '%s' at log position %s in log \ +'%s' produced %ld warning(s). Default database: '%s'", + (char*) table_name, llstr(log_pos,llbuff), RPL_LOG_NAME, - thd->cuted_fields, - (char*)table_name, - print_slave_db_safe((char*)db)); + (ulong) thd->cuted_fields, + print_slave_db_safe(db)); if (net) net->pkt_nr= thd->net.pkt_nr; } @@ -2019,10 +2020,9 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, sql_errno=ER_UNKNOWN_ERROR; err=ER(sql_errno); } - slave_print_error(rli,sql_errno, - "Error '%s' running load data infile \ -(loaded table was '%s', database was '%s')", - err, (char*)table_name, print_slave_db_safe((char*)db)); + slave_print_error(rli,sql_errno,"\ +Error '%s' running lOAD DATA INFILE on table '%s'. Default database: '%s'", + err, (char*)table_name, print_slave_db_safe(db)); free_root(&thd->mem_root,0); return 1; } @@ -2030,10 +2030,9 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, if (thd->fatal_error) { - slave_print_error(rli,ER_UNKNOWN_ERROR, -"Fatal error running \ -LOAD DATA INFILE (loaded table was '%s', database was '%s')", - (char*)table_name, print_slave_db_safe((char*)db)); + slave_print_error(rli,ER_UNKNOWN_ERROR, "\ +Fatal error running LOAD DATA INFILE on table '%s'. Default database: '%s'", + (char*)table_name, print_slave_db_safe(db)); return 1; } @@ -2339,13 +2338,15 @@ int Execute_load_log_event::exec_event(struct st_relay_log_info* rli) What we want instead is add the filename to the current error message. */ char *tmp= my_strdup(rli->last_slave_error,MYF(MY_WME)); - if (!tmp) - goto err; - slave_print_error(rli,rli->last_slave_errno, /* ok to re-use the error code */ - "%s. Failed executing load from '%s'", - tmp, fname); - my_free(tmp,MYF(0)); - thd->options = save_options; + if (tmp) + { + slave_print_error(rli, + rli->last_slave_errno, /* ok to re-use error code */ + "%s. Failed executing load from '%s'", + tmp, fname); + my_free(tmp,MYF(0)); + } + thd->options= save_options; goto err; } thd->options = save_options; diff --git a/sql/mini_client.cc b/sql/mini_client.cc index 1b625a81b0c..a8a0c639abd 100644 --- a/sql/mini_client.cc +++ b/sql/mini_client.cc @@ -30,8 +30,8 @@ #include #include #include -#include #include +#include #include #include #include diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 41807285acc..d9ba97ad11a 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -2506,6 +2506,19 @@ int mysql_service(void *p) return 0; } + +/* Quote string if it contains space, else copy */ + +static char *add_quoted_string(char *to, const char *from, char *to_end) +{ + uint length= (uint) (to_end-to); + + if (!strchr(from, ' ')) + return strnmov(to, from, length); + return strxnmov(to, length, "\"", from, "\"", NullS); +} + + /* Handle basic handling of services, like installation and removal @@ -2515,25 +2528,41 @@ int mysql_service(void *p) servicename Internal name of service displayname Display name of service (in taskbar ?) file_path Path to this program + startup_option Startup option to mysqld RETURN VALUES 0 option handled 1 Could not handle option */ -bool default_service_handling(char **argv, - const char *servicename, - const char *displayname, - const char *file_path) +static bool +default_service_handling(char **argv, + const char *servicename, + const char *displayname, + const char *file_path, + const char *extra_opt) { + char path_and_service[FN_REFLEN+FN_REFLEN+32], *pos, *end; + end= path_and_service + sizeof(path_and_service)-1; + + /* We have to quote filename if it contains spaces */ + pos= add_quoted_string(path_and_service, file_path, end); + if (*extra_opt) + { + /* Add (possible quoted) option after file_path */ + *pos++= ' '; + pos= add_quoted_string(pos, extra_opt, end); + } + *pos= 0; // Ensure end null + if (Service.got_service_option(argv, "install")) { - Service.Install(1, servicename, displayname, file_path); + Service.Install(1, servicename, displayname, path_and_service); return 0; } if (Service.got_service_option(argv, "install-manual")) { - Service.Install(0, servicename, displayname, file_path); + Service.Install(0, servicename, displayname, path_and_service); return 0; } if (Service.got_service_option(argv, "remove")) @@ -2560,12 +2589,11 @@ int main(int argc, char **argv) char file_path[FN_REFLEN]; my_path(file_path, argv[0], ""); /* Find name in path */ fn_format(file_path,argv[0],file_path,"",MY_REPLACE_DIR+ - MY_UNPACK_FILENAME+MY_RESOLVE_SYMLINKS+MY_QUOTE_SPACES); - + MY_UNPACK_FILENAME | MY_RESOLVE_SYMLINKS); if (argc == 2) { - if (!default_service_handling(argv,MYSQL_SERVICENAME, MYSQL_SERVICENAME, - file_path)) + if (!default_service_handling(argv, MYSQL_SERVICENAME, MYSQL_SERVICENAME, + file_path, "")) return 0; if (Service.IsService(argv[1])) { @@ -2578,12 +2606,8 @@ int main(int argc, char **argv) } else if (argc == 3) /* install or remove any optional service */ { - /* Add service name after filename */ - uint length=strlen(file_path); - *strxnmov(file_path + length, sizeof(file_path)-length-2, " ", - argv[2], NullS)= '\0'; - - if (!default_service_handling(argv, argv[2], argv[2], file_path)) + if (!default_service_handling(argv, argv[2], argv[2], file_path, + argv[2])) return 0; if (Service.IsService(argv[2])) { @@ -2605,12 +2629,8 @@ int main(int argc, char **argv) Install an optional service with optional config file mysqld --install-manual mysqldopt --defaults-file=c:\miguel\my.ini */ - uint length=strlen(file_path); - char tmp_path[FN_REFLEN]; - fn_format(tmp_path,argv[3],tmp_path,"",MY_QUOTE_SPACES); - *strxnmov(file_path + length, sizeof(file_path)-length-2, " ", - tmp_path, " ", argv[2], NullS)= '\0'; - if (!default_service_handling(argv, argv[2], argv[2], file_path)) + if (!default_service_handling(argv, argv[2], argv[2], file_path, + argv[3])) return 0; } else if (argc == 1 && Service.IsService(MYSQL_SERVICENAME)) diff --git a/sql/slave.cc b/sql/slave.cc index b4fa11f456e..d7c8c6d5fd3 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -828,6 +828,7 @@ static bool sql_slave_killed(THD* thd, RELAY_LOG_INFO* rli) return rli->abort_slave || abort_loop || thd->killed; } + /* Writes an error message to rli->last_slave_error and rli->last_slave_errno (which will be displayed by SHOW SLAVE STATUS), and prints it to stderr. @@ -842,7 +843,7 @@ static bool sql_slave_killed(THD* thd, RELAY_LOG_INFO* rli) RETURN VALUES void - */ +*/ void slave_print_error(RELAY_LOG_INFO* rli, int err_code, const char* msg, ...) { @@ -853,11 +854,11 @@ void slave_print_error(RELAY_LOG_INFO* rli, int err_code, const char* msg, ...) rli->last_slave_errno = err_code; /* If the error string ends with '.', do not add a ',' it would be ugly */ if (rli->last_slave_error[0] && - (rli->last_slave_error[strlen(rli->last_slave_error)-1] == '.')) - sql_print_error("Slave: %s Error_code=%d", rli->last_slave_error, + (*(strend(rli->last_slave_error)-1) == '.')) + sql_print_error("Slave: %s Error_code: %d", rli->last_slave_error, err_code); else - sql_print_error("Slave: %s, error_code=%d", rli->last_slave_error, + sql_print_error("Slave: %s, Error_code: %d", rli->last_slave_error, err_code); } @@ -872,7 +873,7 @@ void skip_load_data_infile(NET* net) } -char* rewrite_db(char* db) +const char *rewrite_db(const char* db) { if (replicate_rewrite_db.is_empty() || !db) return db; @@ -889,13 +890,14 @@ char* rewrite_db(char* db) /* From other comments and tests in code, it looks like - sometimes Query_log_event and Load_log_event can have db==0 + sometimes Query_log_event and Load_log_event can have db == 0 (see rewrite_db() above for example) (cases where this happens are unclear; it may be when the master is 3.23). */ -char* print_slave_db_safe(char* db) + +const char *print_slave_db_safe(const char* db) { - return (db ? rewrite_db(db) : (char*) ""); + return (db ? rewrite_db(db) : ""); } /* @@ -1303,8 +1305,8 @@ file '%s', errno %d)", fname, my_errno); if (init_io_cache(&rli->info_file, info_fd, IO_SIZE*2, READ_CACHE, 0L,0, MYF(MY_WME))) { - sql_print_error("Failed to create a cache on relay log info file (\ -file '%s')", fname); + sql_print_error("Failed to create a cache on relay log info file '%s'", + fname); msg= current_thd->net.last_error; goto err; } @@ -1313,8 +1315,7 @@ file '%s')", fname); if (init_relay_log_pos(rli,NullS,BIN_LOG_HEADER_SIZE,0 /* no data lock */, &msg)) { - sql_print_error("Failed to open the relay log (relay_log_name='FIRST', \ -relay_log_pos=4)"); + sql_print_error("Failed to open the relay log 'FIRST' (relay_log_pos 4"); goto err; } rli->master_log_name[0]= 0; @@ -1330,15 +1331,16 @@ relay_log_pos=4)"); int error=0; if ((info_fd = my_open(fname, O_RDWR|O_BINARY, MYF(MY_WME))) < 0) { - sql_print_error("Failed to open the existing relay log info file (\ -file '%s', errno %d)", fname, my_errno); + sql_print_error("\ +Failed to open the existing relay log info file '%s' (errno %d)", + fname, my_errno); error= 1; } else if (init_io_cache(&rli->info_file, info_fd, IO_SIZE*2, READ_CACHE, 0L, 0, MYF(MY_WME))) { - sql_print_error("Failed to create a cache on relay log info file (\ -file '%s')", fname); + sql_print_error("Failed to create a cache on relay log info file '%s'", + fname); error= 1; } if (error) @@ -1377,8 +1379,8 @@ file '%s')", fname); &msg)) { char llbuf[22]; - sql_print_error("Failed to open the relay log (relay_log_name='%s', \ -relay_log_pos=%s)", rli->relay_log_name, llstr(rli->relay_log_pos, llbuf)); + sql_print_error("Failed to open the relay log '%s' (relay_log_pos %s)", + rli->relay_log_name, llstr(rli->relay_log_pos, llbuf)); goto err; } } diff --git a/sql/slave.h b/sql/slave.h index d1fd54d3c04..42776061545 100644 --- a/sql/slave.h +++ b/sql/slave.h @@ -382,8 +382,8 @@ 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); -char* rewrite_db(char* db); -char* print_slave_db_safe(char* db); +const char *rewrite_db(const char* db); +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); void slave_print_error(RELAY_LOG_INFO* rli, int err_code, const char* msg, ...); diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 3a3de2abf10..877eab74110 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1242,13 +1242,12 @@ static bool update_user_table(THD *thd, const char *host, const char *user, { /* The tables must be marked "updating" so that tables_ok() takes them into - account in tests. + account in tests. It's ok to leave 'updating' set after tables_ok. */ - tables.updating=1; + tables.updating= 1; /* Thanks to bzero, tables.next==0 */ if (!tables_ok(0, &tables)) DBUG_RETURN(0); - tables.updating=0; } #endif @@ -2138,10 +2137,9 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, The tables must be marked "updating" so that tables_ok() takes them into account in tests. */ - tables[0].updating=tables[1].updating=tables[2].updating=1; + tables[0].updating= tables[1].updating= tables[2].updating= 1; if (!tables_ok(0, tables)) DBUG_RETURN(0); - tables[0].updating=tables[1].updating=tables[2].updating=0; } #endif @@ -2319,10 +2317,9 @@ int mysql_grant (THD *thd, const char *db, List &list, The tables must be marked "updating" so that tables_ok() takes them into account in tests. */ - tables[0].updating=tables[1].updating=1; + tables[0].updating= tables[1].updating= 1; if (!tables_ok(0, tables)) DBUG_RETURN(0); - tables[0].updating=tables[1].updating=0; } #endif diff --git a/vio/viosslfactories.c b/vio/viosslfactories.c index 72ac915d14e..69d4f3007b8 100644 --- a/vio/viosslfactories.c +++ b/vio/viosslfactories.c @@ -18,8 +18,8 @@ #ifdef HAVE_OPENSSL -#include #include +#include #include