From 97a913e31c161dbd18f4fcb9cc00f23640497e03 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sun, 16 Nov 2014 13:12:58 +0100 Subject: [PATCH] cleanup: freshen up CREATE SERVER code * pass LEX_STRING's from the parser, don't ignore the length only to strlen later * init LEX::server_options only for SERVER commands, not for every statement * don't put temporary values into a global persistent memroot but really it's just scratching a surface --- sql/sql_lex.cc | 15 --- sql/sql_lex.h | 9 +- sql/sql_parse.cc | 12 +- sql/sql_servers.cc | 170 +++++++++++----------------- sql/sql_servers.h | 4 +- sql/sql_yacc.yy | 41 ++++--- storage/federated/ha_federated.cc | 14 +-- storage/federatedx/ha_federatedx.cc | 14 +-- 8 files changed, 113 insertions(+), 166 deletions(-) diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 3f0e5673736..add94998cd7 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -523,21 +523,6 @@ void lex_start(THD *thd) lex->select_lex.nest_level_base= &lex->unit; lex->allow_sum_func= 0; lex->in_sum_func= NULL; - /* - ok, there must be a better solution for this, long-term - I tried "bzero" in the sql_yacc.yy code, but that for - some reason made the values zero, even if they were set - */ - lex->server_options.server_name= 0; - lex->server_options.server_name_length= 0; - lex->server_options.host= 0; - lex->server_options.db= 0; - lex->server_options.username= 0; - lex->server_options.password= 0; - lex->server_options.scheme= 0; - lex->server_options.socket= 0; - lex->server_options.owner= 0; - lex->server_options.port= -1; lex->is_lex_started= TRUE; lex->used_tables= 0; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 7aa6aae9289..d1aa2464210 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -202,8 +202,13 @@ typedef Mem_root_array Group_list_ptrs; typedef struct st_lex_server_options { long port; - uint server_name_length; - char *server_name, *host, *db, *username, *password, *scheme, *socket, *owner; + LEX_STRING server_name, host, db, username, password, scheme, socket, owner; + void reset(LEX_STRING name) + { + server_name= name; + host= db= username= password= scheme= socket= owner= null_lex_str; + port= -1; + } } LEX_SERVER_OPTIONS; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 436f69aafd9..68c521af33b 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5481,8 +5481,8 @@ create_sp_error: if ((error= create_server(thd, &lex->server_options))) { DBUG_PRINT("info", ("problem creating server <%s>", - lex->server_options.server_name)); - my_error(error, MYF(0), lex->server_options.server_name); + lex->server_options.server_name.str)); + my_error(error, MYF(0), lex->server_options.server_name.str); break; } my_ok(thd, 1); @@ -5500,8 +5500,8 @@ create_sp_error: if ((error= alter_server(thd, &lex->server_options))) { DBUG_PRINT("info", ("problem altering server <%s>", - lex->server_options.server_name)); - my_error(error, MYF(0), lex->server_options.server_name); + lex->server_options.server_name.str)); + my_error(error, MYF(0), lex->server_options.server_name.str); break; } my_ok(thd, 1); @@ -5521,8 +5521,8 @@ create_sp_error: if (! lex->check_exists && err_code == ER_FOREIGN_SERVER_DOESNT_EXIST) { DBUG_PRINT("info", ("problem dropping server %s", - lex->server_options.server_name)); - my_error(err_code, MYF(0), lex->server_options.server_name); + lex->server_options.server_name.str)); + my_error(err_code, MYF(0), lex->server_options.server_name.str); } else { diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index 2b0576ffba9..670ad182e8b 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -64,9 +64,7 @@ static int insert_server_record_into_cache(FOREIGN_SERVER *server); static FOREIGN_SERVER * prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options); /* drop functions */ -static int delete_server_record(TABLE *table, - char *server_name, - size_t server_name_length); +static int delete_server_record(TABLE *table, LEX_STRING *name); static int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options); /* update functions */ @@ -81,8 +79,6 @@ static int update_server_record_in_cache(FOREIGN_SERVER *existing, /* utility functions */ static void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to); - - static uchar *servers_cache_get_key(FOREIGN_SERVER *server, size_t *length, my_bool not_used __attribute__((unused))) { @@ -603,12 +599,10 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) int error; TABLE_LIST tables; TABLE *table; - LEX_STRING name= { server_options->server_name, - server_options->server_name_length }; DBUG_ENTER("drop_server"); DBUG_PRINT("info", ("server name server->server_name %s", - server_options->server_name)); + server_options->server_name.str)); tables.init_one_table("mysql", 5, "servers", 7, "servers", TL_WRITE); @@ -624,12 +618,12 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) goto end; } - error= delete_server_record(table, name.str, name.length); + error= delete_server_record(table, &server_options->server_name); /* close the servers table before we call closed_cached_connection_tables */ close_mysql_tables(thd); - if (close_cached_connection_tables(thd, &name)) + if (close_cached_connection_tables(thd, &server_options->server_name)) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_UNKNOWN_ERROR, "Server connection in use"); @@ -666,19 +660,19 @@ delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) FOREIGN_SERVER *server; DBUG_ENTER("delete_server_record_in_cache"); - DBUG_PRINT("info",("trying to obtain server name %s length %d", - server_options->server_name, - server_options->server_name_length)); + DBUG_PRINT("info",("trying to obtain server name %s length %zu", + server_options->server_name.str, + server_options->server_name.length)); if (!(server= (FOREIGN_SERVER *) my_hash_search(&servers_cache, - (uchar*) server_options->server_name, - server_options->server_name_length))) + (uchar*) server_options->server_name.str, + server_options->server_name.length))) { - DBUG_PRINT("info", ("server_name %s length %d not found!", - server_options->server_name, - server_options->server_name_length)); + DBUG_PRINT("info", ("server_name %s length %zu not found!", + server_options->server_name.str, + server_options->server_name.length)); goto end; } /* @@ -937,8 +931,7 @@ end: */ static int -delete_server_record(TABLE *table, - char *server_name, size_t server_name_length) +delete_server_record(TABLE *table, LEX_STRING *name) { int error; DBUG_ENTER("delete_server_record"); @@ -946,7 +939,7 @@ delete_server_record(TABLE *table, table->use_all_columns(); /* set the field that's the PK to the value we're looking for */ - table->field[0]->store(server_name, server_name_length, system_charset_info); + table->field[0]->store(name->str, name->length, system_charset_info); if ((error= table->file->ha_index_read_idx_map(table->record[0], 0, (uchar *)table->field[0]->ptr, @@ -989,13 +982,13 @@ int create_server(THD *thd, LEX_SERVER_OPTIONS *server_options) DBUG_ENTER("create_server"); DBUG_PRINT("info", ("server_options->server_name %s", - server_options->server_name)); + server_options->server_name.str)); mysql_rwlock_wrlock(&THR_LOCK_servers); /* hit the memory first */ - if (my_hash_search(&servers_cache, (uchar*) server_options->server_name, - server_options->server_name_length)) + if (my_hash_search(&servers_cache, (uchar*) server_options->server_name.str, + server_options->server_name.length)) goto end; @@ -1034,31 +1027,26 @@ end: int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options) { int error= ER_FOREIGN_SERVER_DOESNT_EXIST; - FOREIGN_SERVER *altered, *existing; - LEX_STRING name= { server_options->server_name, - server_options->server_name_length }; + FOREIGN_SERVER altered, *existing; DBUG_ENTER("alter_server"); DBUG_PRINT("info", ("server_options->server_name %s", - server_options->server_name)); + server_options->server_name.str)); mysql_rwlock_wrlock(&THR_LOCK_servers); if (!(existing= (FOREIGN_SERVER *) my_hash_search(&servers_cache, - (uchar*) name.str, - name.length))) + (uchar*) server_options->server_name.str, + server_options->server_name.length))) goto end; - altered= (FOREIGN_SERVER *)alloc_root(&mem, - sizeof(FOREIGN_SERVER)); + prepare_server_struct_for_update(server_options, existing, &altered); - prepare_server_struct_for_update(server_options, existing, altered); - - error= update_server(thd, existing, altered); + error= update_server(thd, existing, &altered); /* close the servers table before we call closed_cached_connection_tables */ close_mysql_tables(thd); - if (close_cached_connection_tables(thd, &name)) + if (close_cached_connection_tables(thd, &server_options->server_name)) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_UNKNOWN_ERROR, "Server connection in use"); @@ -1089,50 +1077,37 @@ end: static FOREIGN_SERVER * prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options) { - char *unset_ptr= (char*)""; FOREIGN_SERVER *server; DBUG_ENTER("prepare_server_struct"); if (!(server= (FOREIGN_SERVER *)alloc_root(&mem, sizeof(FOREIGN_SERVER)))) DBUG_RETURN(NULL); /* purecov: inspected */ - /* these two MUST be set */ - if (!(server->server_name= strdup_root(&mem, server_options->server_name))) - DBUG_RETURN(NULL); /* purecov: inspected */ - server->server_name_length= server_options->server_name_length; +#define SET_SERVER_OR_RETURN(X, DEFAULT) \ + do { \ + if (!(server->X= server_options->X.str ? \ + strmake_root(&mem, server_options->X.str, \ + server_options->X.length) : "")) \ + DBUG_RETURN(NULL); \ + } while(0) - if (!(server->host= server_options->host ? - strdup_root(&mem, server_options->host) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ + /* name and scheme are always set (the parser guarantees it) */ + SET_SERVER_OR_RETURN(server_name, NULL); + SET_SERVER_OR_RETURN(scheme, NULL); - if (!(server->db= server_options->db ? - strdup_root(&mem, server_options->db) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ + SET_SERVER_OR_RETURN(host, ""); + SET_SERVER_OR_RETURN(db, ""); + SET_SERVER_OR_RETURN(username, ""); + SET_SERVER_OR_RETURN(password, ""); + SET_SERVER_OR_RETURN(socket, ""); + SET_SERVER_OR_RETURN(owner, ""); - if (!(server->username= server_options->username ? - strdup_root(&mem, server_options->username) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ - - if (!(server->password= server_options->password ? - strdup_root(&mem, server_options->password) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ + server->server_name_length= server_options->server_name.length; /* set to 0 if not specified */ server->port= server_options->port > -1 ? server_options->port : 0; - if (!(server->socket= server_options->socket ? - strdup_root(&mem, server_options->socket) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ - - if (!(server->scheme= server_options->scheme ? - strdup_root(&mem, server_options->scheme) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ - - if (!(server->owner= server_options->owner ? - strdup_root(&mem, server_options->owner) : unset_ptr)) - DBUG_RETURN(NULL); /* purecov: inspected */ - DBUG_RETURN(server); } @@ -1156,8 +1131,8 @@ prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, { DBUG_ENTER("prepare_server_struct_for_update"); - altered->server_name= strdup_root(&mem, server_options->server_name); - altered->server_name_length= server_options->server_name_length; + altered->server_name= existing->server_name; + altered->server_name_length= existing->server_name_length; DBUG_PRINT("info", ("existing name %s altered name %s", existing->server_name, altered->server_name)); @@ -1165,23 +1140,21 @@ prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, The logic here is this: is this value set AND is it different than the existing value? */ - altered->host= - (server_options->host && (strcmp(server_options->host, existing->host))) ? - strdup_root(&mem, server_options->host) : 0; +#define SET_ALTERED(X) \ + do { \ + altered->X= \ + (server_options->X.str && strcmp(server_options->X.str, existing->X)) \ + ? strmake_root(&mem, server_options->X.str, server_options->X.length) \ + : 0; \ + } while(0) - altered->db= - (server_options->db && (strcmp(server_options->db, existing->db))) ? - strdup_root(&mem, server_options->db) : 0; - - altered->username= - (server_options->username && - (strcmp(server_options->username, existing->username))) ? - strdup_root(&mem, server_options->username) : 0; - - altered->password= - (server_options->password && - (strcmp(server_options->password, existing->password))) ? - strdup_root(&mem, server_options->password) : 0; + SET_ALTERED(host); + SET_ALTERED(db); + SET_ALTERED(username); + SET_ALTERED(password); + SET_ALTERED(socket); + SET_ALTERED(scheme); + SET_ALTERED(owner); /* port is initialised to -1, so if unset, it will be -1 @@ -1190,21 +1163,6 @@ prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, server_options->port != existing->port) ? server_options->port : -1; - altered->socket= - (server_options->socket && - (strcmp(server_options->socket, existing->socket))) ? - strdup_root(&mem, server_options->socket) : 0; - - altered->scheme= - (server_options->scheme && - (strcmp(server_options->scheme, existing->scheme))) ? - strdup_root(&mem, server_options->scheme) : 0; - - altered->owner= - (server_options->owner && - (strcmp(server_options->owner, existing->owner))) ? - strdup_root(&mem, server_options->owner) : 0; - DBUG_VOID_RETURN; } @@ -1272,13 +1230,13 @@ static FOREIGN_SERVER *clone_server(MEM_ROOT *mem, const FOREIGN_SERVER *server, buffer->server_name_length= server->server_name_length; /* TODO: We need to examine which of these can really be NULL */ - buffer->db= server->db ? strdup_root(mem, server->db) : NULL; - buffer->scheme= server->scheme ? strdup_root(mem, server->scheme) : NULL; - buffer->username= server->username? strdup_root(mem, server->username): NULL; - buffer->password= server->password? strdup_root(mem, server->password): NULL; - buffer->socket= server->socket ? strdup_root(mem, server->socket) : NULL; - buffer->owner= server->owner ? strdup_root(mem, server->owner) : NULL; - buffer->host= server->host ? strdup_root(mem, server->host) : NULL; + buffer->db= safe_strdup_root(mem, server->db); + buffer->scheme= safe_strdup_root(mem, server->scheme); + buffer->username= safe_strdup_root(mem, server->username); + buffer->password= safe_strdup_root(mem, server->password); + buffer->socket= safe_strdup_root(mem, server->socket); + buffer->owner= safe_strdup_root(mem, server->owner); + buffer->host= safe_strdup_root(mem, server->host); DBUG_RETURN(buffer); } diff --git a/sql/sql_servers.h b/sql/sql_servers.h index a6186a85ae2..d5668f0dfcb 100644 --- a/sql/sql_servers.h +++ b/sql/sql_servers.h @@ -26,10 +26,10 @@ typedef struct st_mem_root MEM_ROOT; /* structs */ typedef struct st_federated_server { - char *server_name; + const char *server_name; long port; uint server_name_length; - char *db, *scheme, *username, *password, *socket, *owner, *host, *sport; + const char *db, *scheme, *username, *password, *socket, *owner, *host, *sport; } FOREIGN_SERVER; /* cache handlers */ diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 986305a5420..0aa165fe3f7 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -2494,16 +2494,11 @@ create: ; server_def: - SERVER_SYM - ident_or_text - FOREIGN DATA_SYM WRAPPER_SYM - ident_or_text + SERVER_SYM ident_or_text + { Lex->server_options.reset($2); } + FOREIGN DATA_SYM WRAPPER_SYM ident_or_text OPTIONS_SYM '(' server_options_list ')' - { - Lex->server_options.server_name= $2.str; - Lex->server_options.server_name_length= $2.length; - Lex->server_options.scheme= $6.str; - } + { Lex->server_options.scheme= $7; } ; server_options_list: @@ -2514,27 +2509,33 @@ server_options_list: server_option: USER TEXT_STRING_sys { - Lex->server_options.username= $2.str; + MYSQL_YYABORT_UNLESS(Lex->server_options.username.str == 0); + Lex->server_options.username= $2; } | HOST_SYM TEXT_STRING_sys { - Lex->server_options.host= $2.str; + MYSQL_YYABORT_UNLESS(Lex->server_options.host.str == 0); + Lex->server_options.host= $2; } | DATABASE TEXT_STRING_sys { - Lex->server_options.db= $2.str; + MYSQL_YYABORT_UNLESS(Lex->server_options.db.str == 0); + Lex->server_options.db= $2; } | OWNER_SYM TEXT_STRING_sys { - Lex->server_options.owner= $2.str; + MYSQL_YYABORT_UNLESS(Lex->server_options.owner.str == 0); + Lex->server_options.owner= $2; } | PASSWORD TEXT_STRING_sys { - Lex->server_options.password= $2.str; + MYSQL_YYABORT_UNLESS(Lex->server_options.password.str == 0); + Lex->server_options.password= $2; } | SOCKET_SYM TEXT_STRING_sys { - Lex->server_options.socket= $2.str; + MYSQL_YYABORT_UNLESS(Lex->server_options.socket.str == 0); + Lex->server_options.socket= $2; } | PORT_SYM ulong_num { @@ -7243,13 +7244,12 @@ alter: LEX *lex= Lex; lex->alter_tablespace_info->ts_cmd_type= ALTER_ACCESS_MODE_TABLESPACE; } - | ALTER SERVER_SYM ident_or_text OPTIONS_SYM '(' server_options_list ')' + | ALTER SERVER_SYM ident_or_text { LEX *lex= Lex; lex->sql_command= SQLCOM_ALTER_SERVER; - lex->server_options.server_name= $3.str; - lex->server_options.server_name_length= $3.length; - } + lex->server_options.reset($3); + } OPTIONS_SYM '(' server_options_list ')' { } ; ev_alter_on_schedule_completion: @@ -11863,8 +11863,7 @@ drop: { Lex->sql_command = SQLCOM_DROP_SERVER; Lex->check_exists= $3; - Lex->server_options.server_name= $4.str; - Lex->server_options.server_name_length= $4.length; + Lex->server_options.reset($4); } ; diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index 7b1fa080080..8670048e9a2 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -626,17 +626,17 @@ int get_connection(MEM_ROOT *mem_root, FEDERATED_SHARE *share) at the address of the share. */ share->server_name_length= server->server_name_length; - share->server_name= server->server_name; - share->username= server->username; - share->password= server->password; - share->database= server->db; + share->server_name= const_cast(server->server_name); + share->username= const_cast(server->username); + share->password= const_cast(server->password); + share->database= const_cast(server->db); share->port= server->port > MIN_PORT && server->port < 65536 ? (ushort) server->port : MYSQL_PORT; - share->hostname= server->host; - if (!(share->socket= server->socket) && + share->hostname= const_cast(server->host); + if (!(share->socket= const_cast(server->socket)) && !strcmp(share->hostname, my_localhost)) share->socket= (char *) MYSQL_UNIX_ADDR; - share->scheme= server->scheme; + share->scheme= const_cast(server->scheme); DBUG_PRINT("info", ("share->username %s", share->username)); DBUG_PRINT("info", ("share->password %s", share->password)); diff --git a/storage/federatedx/ha_federatedx.cc b/storage/federatedx/ha_federatedx.cc index 917e5a019ee..5c856ba3ac3 100644 --- a/storage/federatedx/ha_federatedx.cc +++ b/storage/federatedx/ha_federatedx.cc @@ -566,17 +566,17 @@ int get_connection(MEM_ROOT *mem_root, FEDERATEDX_SHARE *share) at the address of the share. */ share->server_name_length= server->server_name_length; - share->server_name= server->server_name; - share->username= server->username; - share->password= server->password; - share->database= server->db; + share->server_name= const_cast(server->server_name); + share->username= const_cast(server->username); + share->password= const_cast(server->password); + share->database= const_cast(server->db); share->port= server->port > MIN_PORT && server->port < 65536 ? (ushort) server->port : MYSQL_PORT; - share->hostname= server->host; - if (!(share->socket= server->socket) && + share->hostname= const_cast(server->host); + if (!(share->socket= const_cast(server->socket)) && !strcmp(share->hostname, my_localhost)) share->socket= (char *) MYSQL_UNIX_ADDR; - share->scheme= server->scheme; + share->scheme= const_cast(server->scheme); DBUG_PRINT("info", ("share->username: %s", share->username)); DBUG_PRINT("info", ("share->password: %s", share->password));