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