maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,

On Mon, Feb 13, 2017 at 03:47:57AM +0530, Varun wrote:
> revision-id: 13ebc9c826de080e416071b7881585b45922bef9 (mariadb-10.2.3-185-g13ebc9c)
> parent(s): 3ae038b732ce503fb839e9095355e05f5c6866f9
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2017-02-13 03:45:59 +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
> 
> ---
>  storage/archive/ha_archive.cc | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc
> index 9d6d100..329e3d5 100644
> --- a/storage/archive/ha_archive.cc
> +++ b/storage/archive/ha_archive.cc
> @@ -372,10 +372,31 @@ int Archive_share::write_v1_metafile()
>  
>  unsigned int ha_archive::pack_row_v1(uchar *record)
>  {
> +  uint offset= 0,length;
>    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)
> +    {
> +      uint start= (fld->ptr - table->record[0]);
> +      uint index_end= start+fld->field_length;
There is no reason to compute the above variables when the field value is an
SQL NULL.
Please move them into the if.
Please also use ptrdiff_t datatype instead of uint.

A different question: did you check what happens when one attempts to write an
SQL NULL value?

> +
> +      if(!(fld->is_real_null(record - table->record[0])))
> +      {
> +        Field_varstring *const field_var = (Field_varstring *)fld;
> +        offset = field_var->data_length() + field_var->length_size();
Coding style says there should be no space before the assignment. Please fix
the above two lines.

> +
> +        for(uint index= start + offset; index <= index_end ; index++)
> +            record_buffer->buffer[index]=0;

Is there any reason you're not using bzero or memset call here instead? (please
check with Serg which one is guaranteed to be available)

> +      }
> +    }
> +  }
>    pos= record_buffer->buffer + table->s->reclength;
>    for (blob= table->s->blob_field, end= blob + table->s->blob_fields;
>         blob != end; blob++)

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog