On Wed, 9 Aug 2023 13:53:38 -0400
Stevie Alvarez <stevie.6strings@xxxxxxxxx> wrote:
> +++ b/src/histograms.c
> @@ -688,3 +688,117 @@ void traceeval_results_release(struct traceeval *teval,
>
> data_release(teval->nr_val_types, &results, teval->val_types);
> }
> +
> +/*
> + * Create a new entry in @teval with respect to @keys and @vals.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +static int create_entry(struct traceeval *teval,
> + const union traceeval_data *keys,
> + const union traceeval_data *vals)
> +{
> + union traceeval_data *new_keys;
> + union traceeval_data *new_vals;
> + struct entry *tmp_map;
> + struct hist_table *hist = teval->hist;
> +
> + /* copy keys */
> + if (copy_traceeval_data_set(teval->nr_key_types, teval->key_types,
> + keys, &new_keys) == -1)
> + return -1;
> +
> + /* copy vals */
> + if (copy_traceeval_data_set(teval->nr_val_types, teval->val_types,
> + vals, &new_vals) == -1)
> + goto fail_vals;
> +
> + /* create new entry */
> + tmp_map = realloc(hist->map, ++hist->nr_entries * sizeof(*tmp_map));
The "++hist->nr_entries" is called a side effect. Where the allocation is
adding more functionality than expected. I try to avoid this even though it
may seem to be clever. One reason is they tend to introduce bugs. For
example, if the allocation fails, the nr_entries now has more entries
then what exists.
I only would update nr_entries on a allocation success.
-- Steve
> + if (!tmp_map)
> + goto fail;
> + tmp_map->keys = new_keys;
> + tmp_map->vals = new_vals;
> + hist->map = tmp_map;
> +
> + return 0;
> +
> +fail:
> + data_release(teval->nr_val_types, &new_vals, teval->val_types);
> +
> +fail_vals:
> + data_release(teval->nr_key_types, &new_keys, teval->key_types);
> + return -1;
> +}
> +
|