Yuchen Pei added a comment - 2025-01-14 03:13 Hi danblack , ptal thanks: 0432a765de0 upstream/bb-10.5-mdev-35840 MDEV-35840 Eliminate -warray-bounds triggered by TABLE_SHARE::db_type() Old buildbot pages: https://buildbot.mariadb.net/buildbot/changes/808821 https://buildbot.mariadb.net/buildbot/changes/808823

Marko Mäkelä added a comment - 2025-01-14 05:13 - edited I’m glad to see some progress here. The following is reducing some but not all repetition: diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 6228991d46c..73742a1da44 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -721,9 +721,10 @@ static bool handle_list_of_fields(THD *thd, List_iterator<const char> it, } else { - if (table->s->db_type()->partition_flags && - (table->s->db_type()->partition_flags() & HA_USE_AUTO_PARTITION) && - (table->s->db_type()->partition_flags() & HA_CAN_PARTITION)) + handlerton *ht= table->s->db_type(); + if (ht->partition_flags && + (ht->partition_flags() & HA_USE_AUTO_PARTITION) && + (ht->partition_flags() & HA_CAN_PARTITION)) { /* This engine can handle automatic partitioning and there is no We could test both flags at once with a single call and a single bitwise-and operation: if (ht->partition_flags && !~(ht->partition_flags() & (HA_USE_AUTO_PARTITION | HT_CAN_PARTITION))) Also, it seems that the ternary operator could be used here: @@ -4927,10 +4932,11 @@ uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info, if default partitioning is used. */ + handlerton *ht= table->s->db_type(); if (tab_part_info->part_type != HASH_PARTITION || - ((table->s->db_type()->partition_flags() & HA_USE_AUTO_PARTITION) && + ((ht->partition_flags() & HA_USE_AUTO_PARTITION) && !tab_part_info->use_default_num_partitions) || - ((!(table->s->db_type()->partition_flags() & HA_USE_AUTO_PARTITION)) && + ((!(ht->partition_flags() & HA_USE_AUTO_PARTITION)) && tab_part_info->use_default_num_partitions)) { my_error(ER_REORG_NO_PARAM_ERROR, MYF(0)); That is, (a && b) || (!a || c) could be better written as a ? b : c , to avoid evaluating a twice. In fact, here we would seem to have c = !b and therefore this would seem to be an equivalence operation. While the bitwise exclusive-or operator ^ has no logical counterpart (double caret), the equality comparison == comes close, that is: !(table->s->db_type()->partition_flags() & HA_USE_AUTO_PARTITION) == !!tab_part_info->use_default_num_partitions The double negation !! will normalize 0 to false and nonzero to true .

Yuchen Pei added a comment - 2025-01-14 06:41 - edited Thanks for the review comments marko and danblack . Updated the commit - ptal thanks: 5ef712bdced bb-10.5-mdev-35840 MDEV-35840 Eliminate -warray-bounds triggered by TABLE_SHARE::db_type()

Marko Mäkelä added a comment - 2025-01-14 06:53 For clarification: I had misplaced the parenthesis in my previous suggestion: if (ht->partition_flags && !(~ht->partition_flags() & (HA_USE_AUTO_PARTITION | HT_CAN_PARTITION))) danblack suggested the more readable variant if (ht->partition_flags && (ht->partition_flags() & (HA_USE_AUTO_PARTITION | HT_CAN_PARTITION)) == (HA_USE_AUTO_PARTITION | HT_CAN_PARTITION))) According to https://godbolt.org , only starting with clang 13 or GCC 13 targeting AMD64 at -O2 level they translate to the equivalent code. Older compiler versions actually emit code for both the bitwise and and the equality cmp . Later ones emit not and test , which my suggestion would always compile to. In any case, either variant is better than the original, which seems to have duplicated the function call.

Daniel Black added a comment - 2025-01-15 04:19 Looks good ycp , good to merge.

Yuchen Pei added a comment - 2025-01-16 00:20 thanks for the review - pushed 78157c4765f2c086fabe183d51d7734ecffdbdd8 to 10.5