maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,


Please
* fix the commit comment
* add a comment
* fix the coding style 

as indicated below.

(and also please conform to the coding style in the future. If its requirements
are not clear, lets sit down once and discuss them)

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

What "NULL values"?? The fix is filling with zeros the space beyond the end of
VARCHAR values. VARCHAR NULLs are actually ok already as NULL value has its buffer 
filled with zeros.

Please come up with a comment that describes the ix.
> 
> ---
>  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;
>    uint *blob, *end;
>    uchar *pos;
>    DBUG_ENTER("pack_row_v1");
>    memcpy(record_buffer->buffer, record, table->s->reclength);
Please add here a comment about what this loop is doing.

> +  for(Field** field= table->field; (*field) ; field++)
space needed in 'for ('

> +  {
> +    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();
> +        bzero(record_buffer->buffer + start + offset, fld->field_length - offset + 1);
The above line is longer than 80 chars. should be broken into:

        bzero(record_buffer->buffer + start + offset, 
              fld->field_length - offset + 1);


> +      }
> +    }
> +  }
>    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