Destructor callback for reference

[nokute78]

Background

cfl_variant_destroy destroys if a variant is string, bytes, array or kvlist.
It doesn't if the type is reference.

cfl_array and cfl_kvlist supports reference.
It means that user needs to release before destroying them.

e.g.

        for (index = 0 ; index < array->entry_count ; index++) {
            if(array->entries[index] != NULL && array->entries[index].type == CFL_VARIANT_REFERENCE) {
                free(array->entries[index].data.as_reference);
            }
        }
        cfl_array_destroy(array);

We need to save keys to fetch references.

        for (index = 0 ; index < keys_size ; index++) {
              var = cfl_kvlist_fetch(kvlist, keys[index]);
              if (var != NULL && var.type == CFL_VARIANT_REFERENCE ) {
                   free(var.data.as_reference)
              }
        }
        cfl_kvlist_destroy(kvlist);

I think it is messy.

Proposal

I propose to add a function pointer to struct cfl_variant and add a new function cfl_variant_set_destructor(funcptr).

cfl_variant_destroy calls funcptr if it is not NULL.
It allows user to register destructor callback.

cfl_kvlist_insert_reference and cfl_array_append_reference will be extracted to pass funcptr.

Note: Do not set callback on creating

We can extract like cfl_variant_create_from_reference(value, funcptr).
However it causes issue following case.
The reference will be released when cfl_array_append is failed.

int cfl_array_append_reference(struct cfl_array *array, void *value, void (*funcptr) (void *))
{
    struct cfl_variant *value_instance;
    int                 result;

    value_instance = cfl_variant_create_from_reference(value, funcptr); // set callback on create

    if (value_instance == NULL) {
        return -1;
    }

    result = cfl_array_append(array, value_instance);

    if (result) {
        cfl_variant_destroy(value_instance); // reference is released !

        return -2;
    }

    return 0;
}

[edsiper]

it seems the only user of this reference data type is the attributes processor. wondering the use case that justify this api change, do we need it ?

[nokute78]

Currently there is no use cases. There is no urgency.

Fluent-bit uses cfl_kvlist/array for config_format.
Once it contains a reference, we will need to add releasing codes before calling cfl_kvlist_destroy/cfl_array_destroy like

        for (index = 0 ; index < keys_size ; index++) {
              var = cfl_kvlist_fetch(kvlist, keys[index]);
              if (var != NULL && var.type == CFL_VARIANT_REFERENCE ) {
                   free(var.data.as_reference)
              }
        }
        cfl_kvlist_destroy(kvlist);

It will affect following code.
https://github.com/fluent/fluent-bit/blob/v2.2.2/src/config_format/flb_cf_yaml.c#L591

[edsiper]

so I think we should close it since its not needed