On Fri, 4 Aug 2023 14:23:11 -0400
Stevie Alvarez <stevie.6strings@xxxxxxxxx> wrote:
> > > + * traceeval_init - create a traceeval descriptor
> > > + * @keys: Defines the keys to differentiate traceeval entries
> > > + * @vals: Defines values attached to entries differentiated by @keys.
> > > + *
> > > + * The @keys and @vals define how the traceeval instance will be populated.
> > > + * The @keys will be used by traceeval_query() to find an instance within
> > > + * the "histogram". Note, both the @keys and @vals array must end with:
> > > + * { .type = TRACEEVAL_TYPE_NONE }.
> > > + *
> > > + * The @keys and @vals passed in are copied for internal use.
> > > + *
> > > + * For any member of @keys or @vals that isn't of type TRACEEVAL_TYPE_NONE,
> > > + * the name field must be a null-terminated string. For members of type
> > > + * TRACEEVAL_TYPE_NONE, the name is ignored.
> > > + *
> > > + * @vals can be NULL or start with its type field as TRACEEVAL_TYPE_NONE to
> > > + * define the values of the histogram to be empty.
> > > + * @keys must be populated with at least one element that is not
> > > + * TRACEEVAL_TYPE_NONE.
> > > + *
> > > + * Returns the descriptor on success, or NULL on error.
> > > + */
> > > +struct traceeval *traceeval_init(const struct traceeval_type *keys,
> > > + const struct traceeval_type *vals)
> > > +{
> > > + struct traceeval *teval;
> > > + char *err_msg;
> > > +
> > > + if (!keys)
> > > + return NULL;
> > > +
> > > + if (keys->type == TRACEEVAL_TYPE_NONE) {
> > > + err_msg = "keys cannot start with type TRACEEVAL_TYPE_NONE";
> >
> > BTW, all the error messages should start with a capital letter.
> >
> > "Keys cannot .."
> >
> > > + goto fail_init_unalloced;
> >
> > Just make this: goto fail;
>
> Having multiple of the same label segfaults my tests. I'm relatively new
> to using C labels, is that behavior expected? And if so is it acceptable
> for me to distingush the two fail labels by a single character?
labels must be unique per function. But I constantly use the same label
multiple times per C file.
As I stated in the email, convert one to "fail:" and the other one to "fail_release:"
>
> -- Stevie
>
> >
> > > + }
> > > +
> > > + /* alloc teval */
> > > + teval = calloc(1, sizeof(*teval));
> > > + if (!teval) {
> > > + err_msg = "failed to allocate memory for traceeval instance";
> > > + goto fail_init_unalloced;
> > > + }
> > > +
> > > + /* alloc key types */
> > > + teval->nr_key_types = type_alloc(keys, &teval->key_types);
> > > + if (teval->nr_key_types <= 0) {
> > > + teval->nr_key_types *= -1;
> >
> > Get rid of the above setting.
> >
> > > + err_msg = "failed to allocate user defined keys";
> > > + goto fail_init;
> >
> > and this: goto fail_release;
> >
> > > + }
> > > +
> > > + /* alloc val types */
> > > + teval->nr_val_types = type_alloc(vals, &teval->val_types);
> > > + if (teval->nr_val_types < 0) {
> > > + teval->nr_val_types *= -1;
> >
> > And git rid of the above too.
> >
> > > + err_msg = "failed to allocate user defined values";
> > > + goto fail_init;
> > > + }
> > > +
> > > + /* alloc hist */
> > > + teval->hist = calloc(1, sizeof(*teval->hist));
> > > + if (!teval->hist) {
> > > + err_msg = "failed to allocate memory for histogram";
> > > + goto fail_init;
> > > + }
> > > +
> > > + return teval;
> > > +
> > > +fail_init:
This would be
fail_release:
as it releases the teval.
> > > + traceeval_release(teval);
> >
> > The above isn't defined. If you reference a function in a patch, it must be
> > at least defined.
> >
> > > +
> > > +fail_init_unalloced:
and this one be
fail:
As it is part of the fail path for both.
-- Steve
> > > + print_err(err_msg);
> > > + return NULL;
> > > +}
> >
|