Hi, Oleksandr!
A couple of minor comments, see below
On Jul 15, Oleksandr Byelkin wrote:
revision-id: d7b274fa257 (mariadb-10.2.25-63-gd7b274fa257)
parent(s): 64900e3d7c3
author: Oleksandr Byelkin <sanja@mariadb.com>
committer: Oleksandr Byelkin <sanja@mariadb.com>
timestamp: 2019-07-11 13:29:51 +0200
message:
MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
Make automatic name generation during execution (not prepare).
Check result of memory allocation operation.
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 043cfaaaaaa..636fb9f427c 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -6205,6 +6210,10 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info)
Virtual_column_info *check;
TABLE_SHARE *share= table->s;
uint c;
+
+ if (fix_constraints_names(thd, &alter_info->check_constraint_list))
+ DBUG_RETURN(TRUE);
It's not very logical, I think, to generate constraint names in
handle_if_exists_options(), I'd rather move it out and put directly in
mysql_alter_table().
+
while ((check=it++))
{
if (!(check->flags & Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS) &&
@@ -6232,10 +6241,44 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info)
}
}
- DBUG_VOID_RETURN;
+ DBUG_RETURN(FALSE);
}
+static bool fix_constraints_names(THD *thd, List<Virtual_column_info>
+ *check_constraint_list)
+{
+ List_iterator<Virtual_column_info> it((*check_constraint_list));
+ Virtual_column_info *check;
+ uint nr= 1;
+ DBUG_ENTER("fix_constraints_names");
+ if (!check_constraint_list)
+ DBUG_RETURN(FALSE);
+ // Prevent accessing freed memory during generating unique names
+ while ((check=it++))
+ {
+ if (check->automatic_name)
+ {
+ check->name.str= NULL;
+ check->name.length= 0;
+ }
+ }
+ it.rewind();
+ // Grnerate unique names if needed
typo
+ while ((check=it++))
+ {
+ if (!check->name.length)
+ {
+ check->automatic_name= TRUE;
+ if (make_unique_constraint_name(thd, &check->name,
+ check_constraint_list,
+ &nr))
+ DBUG_RETURN(TRUE);
+ }
+ }
+ DBUG_RETURN(FALSE);
I would remove the first while() at all and just do the second one till
the end:
bool ret= FALSE;
while ((check=it++))
if (check->automatic_name)
ret |= make_unique_constraint_name(...);
return ret;
Because thd->strmake() basically never fails, it doesn't make much sense
to optimize for a case when it does.
This code above assumes you set automatic_name=TRUE in
mysql_prepare_create_table().
+}
+
/**
Get Create_field object for newly created table by field index.
Regards,
Sergei
Oleksandr Byelkin
Sergei Golubchik