[MariaDB developers] Re: d2de9a89d80: MDEV-31531 Remove my_casedn_str() and my_caseup_str()

Hi, Alexander, Looks good. A couple of minor comments below On Dec 19, Alexander Barkov wrote:

commit d2de9a89d80 Author: Alexander Barkov Date: Fri Jun 23 13:24:02 2023 +0400

diff --git a/include/m_ctype.h b/include/m_ctype.h index 9d13989d1fe..65ccf869992 100644 --- a/include/m_ctype.h +++ b/include/m_ctype.h @@ -1512,6 +1540,14 @@ size_t my_copy_fix_mb(CHARSET_INFO *cs, /* Functions for 8bit */ extern size_t my_caseup_str_8bit(CHARSET_INFO *, char *); extern size_t my_casedn_str_8bit(CHARSET_INFO *, char *); +static inline size_t my_caseup_str_latin1(char *str)

caseup variant doesn't seem to be used anywhere

+{ + return my_caseup_str_8bit(&my_charset_latin1, str); +} +static inline size_t my_casedn_str_latin1(char *str) +{ + return my_casedn_str_8bit(&my_charset_latin1, str); +} extern size_t my_caseup_8bit(CHARSET_INFO *, const char *src, size_t srclen, char *dst, size_t dstlen); diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 09af5523c76..3166c9e56cf 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2543,6 +2541,37 @@ static void push_new_user(const ACL_USER &user) }

+/** + Make a database name on mem_root from a String, + apply lower-case conversion if lower_case_table_names says so. + Perform database name length limit validation. + + @param thd - the THD, to get the warning text from + @param mem_root - allocate the result on this memory root + @param dbstr - the String, e.g. with Field::val_str() result + + @return - {NULL,0} in case of EOM or a bad database name, + or a good database name otherwise. +*/ +static LEX_STRING make_and_check_db_name(MEM_ROOT *mem_root, + const String &dbstr) +{ + LEX_STRING dbls= lower_case_table_names ? + lex_string_casedn_root(mem_root, files_charset_info, + dbstr.ptr(), dbstr.length()) : + lex_string_strmake_root(mem_root, + dbstr.ptr(), dbstr.length()); + if (!dbls.str) + return LEX_STRING{NULL, 0}; // EOM

what's the difference between this and return {NULL, 0};

+ if (dbls.length > SAFE_NAME_LEN) + { + sql_print_warning(ER_DEFAULT(ER_WRONG_DB_NAME), dbls.str); + return LEX_STRING{NULL, 0}; // Bad name + } + return dbls; // Good name +} + + /* Initialize structures responsible for user/db-level privilege checking and load information about grants from open privilege tables. diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index a84fd29affc..f61e5085e47 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -4186,6 +4187,18 @@ bool LEX::copy_db_to(LEX_CSTRING *to) return thd->copy_db_to(to); }

+ +Lex_ident_db_normalized LEX::copy_db_normalized() +{ + if (sphead && sphead->m_name.str) + { + DBUG_ASSERT(sphead->m_db.str && sphead->m_db.length);

you missed that DBUG_ASSERT(sphead->m_db.str); DBUG_ASSERT(sphead->m_db.length);

+ return thd->to_ident_db_normalized_with_error(sphead->m_db); + } + return thd->copy_db_normalized(); +} + + /** Initialize offset and limit counters.

diff --git a/storage/perfschema/pfs_program.cc b/storage/perfschema/pfs_program.cc index de456610519..f3734cb383b 100644 --- a/storage/perfschema/pfs_program.cc +++ b/storage/perfschema/pfs_program.cc @@ -118,31 +118,21 @@ static void set_program_key(PFS_program_key *key, */

char *ptr= &key->m_hash_key[0]; + const char *end= ptr + sizeof(key->m_hash_key) - 1;

ptr[0]= object_type; ptr++;

if (object_name_length > 0) - { - char tmp_object_name[COL_OBJECT_NAME_SIZE + 1]; - memcpy(tmp_object_name, object_name, object_name_length); - tmp_object_name[object_name_length]= '\0'; - my_casedn_str(system_charset_info, tmp_object_name); - memcpy(ptr, tmp_object_name, object_name_length); - ptr+= object_name_length; - } + ptr+= system_charset_info->casedn(object_name, object_name_length, + ptr, end - ptr); ptr[0]= 0; ptr++;

if (schema_name_length > 0) - { - char tmp_schema_name[COL_OBJECT_SCHEMA_SIZE + 1]; - memcpy(tmp_schema_name, schema_name, schema_name_length); - tmp_schema_name[schema_name_length]='\0'; - my_casedn_str(system_charset_info, tmp_schema_name); - memcpy(ptr, tmp_schema_name, schema_name_length); - ptr+= schema_name_length; - } + ptr+= system_charset_info->opt_casedn(schema_name, schema_name_length, + ptr, end - ptr, + lower_case_table_names);

why system_charset_info and not files_charset_info ?

ptr[0]= 0; ptr++;

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org