Am 15.07.19 um 19:21 schrieb Sergei Golubchik:
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 <sa...@mariadb.com>
committer: Oleksandr Byelkin <sa...@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().
Absolutely agree. I tough to move but decided that it is too radical
change, but if you think so also I will do.
+
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
ok
+ 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.
it is not optimisation it is just bug in error reporting, just several
month ago was fixing bug with the code like this, in any case it is
better to report error than coredump (it is especially important because
we do not have regular testing with low memory).
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
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp