← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 00c30e5: MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value)

 

Hi Varun,

On Wed, Mar 01, 2017 at 07:50:01AM +0530, Varun wrote:
> revision-id: 00c30e5a234fbd01484cd069c01e97c075025323 (mariadb-10.2.3-283-g00c30e5)
> parent(s): b13cee83f9cfcffa98d227c492411a9acec85f42
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2017-03-01 07:46:26 +0530
> message:
> 
> MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value)
> 
> Set null values to record_buffer->buffer for the VARCHAR fields
It would be nice to elaborate where uninitialized data comes from. Am I right
that it comes from record? It would be nice to explain why NULL values and CHAR
type are not affected. As well as why 10.1 is unaffected, ideally point to
revision that caused this.

> 
> ---
>  storage/archive/ha_archive.cc | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc
> index 9d6d100..5599b43 100644
> --- a/storage/archive/ha_archive.cc
> +++ b/storage/archive/ha_archive.cc
> @@ -372,10 +372,25 @@ int Archive_share::write_v1_metafile()
>  
>  unsigned int ha_archive::pack_row_v1(uchar *record)
>  {
> +  uint offset;
Why you declare offset here...

>    uint *blob, *end;
>    uchar *pos;
>    DBUG_ENTER("pack_row_v1");
>    memcpy(record_buffer->buffer, record, table->s->reclength);
> +  for(Field** field= table->field; (*field) ; field++)
> +  {
> +    Field *fld= *field;
> +    if (fld->type() == MYSQL_TYPE_VARCHAR)
> +    {
> +      if (!(fld->is_real_null(record - table->record[0])))
> +      {
> +        ptrdiff_t  start= (fld->ptr - table->record[0]);
> +        Field_varstring *const field_var= (Field_varstring *)fld;
> +        offset= field_var->data_length() + field_var->length_size();
And use it here? Why not declare it inline with start and field_var?

> +        bzero(record_buffer->buffer + start + offset, fld->field_length - offset + 1);
According to man bzero:

  This function is deprecated (marked as LEGACY in POSIX.1-2001): use memset(3)
  in new programs. POSIX.1-2008 removes the specification of bzero().

Please use memset() instead.

Regards,
Sergey