libcyaml icon indicating copy to clipboard operation
libcyaml copied to clipboard

Uniformize load and free function arguments types

Open berthin opened this issue 4 years ago • 1 comments

Hi,

I noticed that cyaml_load_file and cyaml_free have slightly different interfaces but they could basically use the same type for data.

You could have:

cyaml_err_t cyaml_free(
		const cyaml_config_t *config,
		const cyaml_schema_value_t *schema,
		cyaml_data_t **data,
		unsigned seq_count)
{
    ...
}

and, instead of

cyaml_load_file(filename, &settings, &schema, &data, 0)
cyaml_free(&settings, &schema, data, 0);

we can use:

cyaml_load_file(filename, &settings, &schema, &data, 0)
cyaml_free(&settings, &schema, &data, 0);

Best,

berthin avatar Aug 11 '20 09:08 berthin

The cyaml_load_file call has to take a double pointer (cyaml_data_t **data) because it needs to return the pointer to a new allocation to the caller through a function parameter (the function return value being used for an error code).

The cyaml_free call follows the convention of the standard library's free function, and takes a single pointer to the data to be freed.

Any change to this now would change the ABI and require bumping to a 2.0.0 release.

One advantage of changing cyaml_free to take a double pointer, is that it could NULL the data for you, so you could achieve

cyaml_free(&settings, &schema, data, 0);
data = NULL;

with just

cyaml_free(&settings, &schema, &data, 0);

I'll consider it when I have enough other changes to warrant a 2.0.0 release.

Thanks for the feedback!

tlsa avatar Aug 11 '20 09:08 tlsa