maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Oleksandr!

On Dec 21, Oleksandr Byelkin wrote:

> It looks like you reviewed some old sources in 10.0-cassandra this is 
> fixed as we discussed it.

I've reviewed the 5.5-cassandra tree. And yes, I thought that was
already fixed, so I was a bit susprised to find it unfixed :)
Ok, wrong tree, then. Thanks!

> [skip]
> >   
> > -
> > -int dynamic_column_update(DYNAMIC_COLUMN *str, uint column_nr,
> > -                          DYNAMIC_COLUMN_VALUE *value)
> > +enum enum_dyncol_func_result
> > +dynamic_column_vals(DYNAMIC_COLUMN *str,
> > +                    DYNAMIC_ARRAY *names, DYNAMIC_ARRAY *vals,
> > +                    char **free_names)
> >   {
> > -  return dynamic_column_update_many(str, 1, &column_nr, value);
> > +  DYN_HEADER header;
> > +  char *nm;
> > +  uint i;
> > +  enum enum_dyncol_func_result rc;
> > +
> > +  *free_names= 0;
> > +  bzero(names, sizeof(DYNAMIC_ARRAY));        /* In case of errors */
> > +  bzero(vals, sizeof(DYNAMIC_ARRAY));         /* In case of errors */
> > +  if (str->length == 0)
> > +    return ER_DYNCOL_OK;                      /* no columns */
> > +
> > +  if ((rc= init_read_hdr(&header, str)) < 0)
> > +    return rc;
> > +
> > +  if (header.entry_size * header.column_count + FIXED_HEADER_SIZE >
> > +      str->length)
> > +    return ER_DYNCOL_FORMAT;
> > +
> > +  if (init_dynamic_array(names, sizeof(LEX_STRING),
> > +                         header.column_count, 0) ||
> > +      init_dynamic_array(vals, sizeof(DYNAMIC_COLUMN_VALUE),
> > +                         header.column_count, 0) ||
> > +      (header.format == DYNCOL_FMT_NUM &&
> > +       !(*free_names= (char *)malloc(DYNCOL_NUM_CHAR * header.column_count))))
> > why do you need a special malloc() for names, you can put them in
> > the buffer of 'names' dynarray. it'll be a lot more convenient for
> > the caller (no free_names to care about).
> dynamic array also should be freed, so I do not see difference, I can 
> rename it to memory_to_free or memory_should_be_freed.

The difference is - you pass a separate pointer to the caller, that a
caller should take care of. If you put this memory in dynarray, than the
caller doesn't have to bother. He eventually delete_dynamic() and that
does all the necessary cleanup. It's easier to use for the caller - and
for API it's extremely important. More important than simple
implementation.

Regards,
Sergei