maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Varun!

Oh, I see :)

I almost replied to your email with "it's impossible, Archive did not
have HA_RECORD_MUST_BE_CLEAN_ON_WRITE in the table_flags(), no engine
did. So removal could not have changed anything".

But then I noticed that the code - by some old typo - had "&&" not "&".
So it was always doing restore_record() in valgrind-aware binaries.
For all engines. Apparently, it was never the original intention.

Note that in release binaries HAVE_valgrind is not defined, and the
record is not bzero-ed even in 10.1 and earlier versions. This is a bug
in Archive engine, in all versions, not in 10.2 only. In should not
store the uninitialized tail of VARCHAR columns.

On Feb 07, Varun Gupta wrote:
> Hi,
> 
> The commit is:
> 
> commit c697ddc315429c56dec567b2fe85cfe5c03834ea
> Author: Sergei Golubchik <serg@xxxxxxxxxxx>
> Date:   Mon Dec 12 15:47:51 2016 +0100
> 
>     cleanup: remove unused handler table flag
> 
> diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
> 
> index 901fd48..9c21cb7 100644
> --- a/sql/sql_insert.cc
> +++ b/sql/sql_insert.cc
> 
> @@ -955,12 +955,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
>              be overwritten by fill_record() anyway (and fill_record() does
> not
>              use default values in this case).
>            */
> -#ifdef HAVE_valgrind
> -          if (table->file->ha_table_flags() && HA_RECORD_MUST_BE_CLEAN_ON_WRITE)
> -            restore_record(table,s->default_values);   // Get empty record
> -          else
> -#endif
> -            table->record[0][0]= share->default_values[0];
> +          table->record[0][0]= share->default_values[0];
> 
> On Sun, Feb 5, 2017 at 8:10 PM, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> 
> > Hi, Varun!
> >
> > On Feb 05, Varun Gupta wrote:
> > > Hi,
> > >
> > > Here is the issue https://jira.mariadb.org/browse/MDEV-11645 on which I
> > > need suggestions
> > >
> > > The observations made are:
> > >
> > > - in 10.1, varchar endspace was bzero-ed
> > > archive depended on it to not pack garbage
> > >
> > > - in 10.2, varchar endspace is not bzero-ed
> > >  which breaks the archive SE.
> > >
> > > Was this change intended?
> > >
> > > If yes, then we should update ha_archive to fill the garbage parts
> > > of the buffer with zeroes that will cause it to be (slightly?)
> > > slower  but there's no other way.
> >
> > Could you find what commit caused it? git bisect is quite useful for
> > that.
> >
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx