maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Michael!

On Mar 29, Michael Widenius wrote:
> revision-id: 1bbc852ef71 (mariadb-10.5.2-525-g1bbc852ef71)
> parent(s): 039f4a054bb
> author: Michael Widenius <michael.widenius@xxxxxxxxx>
> committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> timestamp: 2021-03-24 14:31:54 +0200
> message:
> 
> Changed field_index to use field_index_t instead of uint16

Just curious. Why?

> diff --git a/sql/unireg.cc b/sql/unireg.cc
> index 51dd1fb6e8c..f1a0c5c5269 100644
> --- a/sql/unireg.cc
> +++ b/sql/unireg.cc
> @@ -146,8 +146,8 @@ get_fieldno_by_name(HA_CREATE_INFO *create_info, List<Create_field> &create_fiel
>    {
>      if (field_name.streq(sql_field->field_name))
>      {
> -      DBUG_ASSERT(field_no <= uint16(~0U));
> +      DBUG_ASSERT(field_no < NO_CACHED_FIELD_INDEX);

this look suspicios. NO_CACHED_FIELD_INDEX is ((uint)(-1))
which is always greater than anything you can put into field_index_t.

You want to ensure that field_no is valid when casted into
field_index_t. I'd suggest to redefine NO_CACHED_FIELD_INDEX as

    ((field_index_t)~0U)

> -      return uint16(field_no);
> +      return (field_index_t) field_no;

why not to declare field_no to be field_index_t? then you won't need a
cast.

>      }
>    }
>  
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx