On Mon, 19 May 2014 23:29:35 +0900
Namhyung Kim <namhy...@kernel.org> wrote:
> > - * struct plugin_option PEVENT_PLUGIN_OPTIONS[] = {
> > + * struct pevent_plugin_option PEVENT_PLUGIN_OPTIONS[] = {
> > * {
> > * .name = "option-name",
> > * .plugin_alias = "overide-file-name", (optional)
> > @@ -355,7 +355,7 @@ enum pevent_func_arg_type {
> > enum pevent_flag {
> > PEVENT_NSEC_OUTPUT = 1, /* output in NSECS */
> > PEVENT_DISABLE_SYS_PLUGINS = 1 << 1,
> > - PEVENT_DISABLE_PLUGINS = 1 << 2,
> > + PEVENT_DISABLE_PLUGINS = 1 << 2
>
> Unnecessary change?
Hmm, no idea why I changed that.
> > +/**
> > + * traceevent_plugin_list_options - get list of plugin options
> > + *
> > + * Returns an array of char strings that list the currently registered
> > + * plugin options in the format of <plugin>:<option>. This list can be
> > + * used by toggling the option.
> > + *
> > + * Returns NULL if there's no options registered. On error it returns
> > + * an (char **)-1 (must check for that)
>
> What about making it a macro like INVALID_OPTION_LIST?
Yeah, I could do this.
>
> > + *
> > + * Must be freed with traceevent_plugin_free_options_list().
> > + */
> > +char **traceevent_plugin_list_options(void)
> > +{
> > + struct registered_plugin_options *reg;
> > + struct pevent_plugin_option *op;
> > + char **list = NULL;
> > + char *name;
> > + int count = 0;
> > +
> > + for (reg = registered_options; reg; reg = reg->next) {
> > + for (op = reg->options; op->name; op++) {
> > + char *alias = op->plugin_alias ? op->plugin_alias :
> > op->file;
> > +
> > + name = malloc(strlen(op->name) + strlen(alias) + 2);
> > + if (!name)
> > + goto err;
> > +
> > + sprintf(name, "%s:%s", alias, op->name);
> > + list = realloc(list, count + 2);
> > + if (!list) {
>
> This will lost the original list pointer. Please use a temp variable.
Will fix.
>
>
> > + free(name);
> > + goto err;
> > + }
> > + list[count++] = name;
> > + list[count] = NULL;
> > + }
> > + }
> > + if (!count)
> > + return NULL;
> > + return list;
>
> Isn't it enough to simply return the list?
Yep, will do.
>
> > +
> > + err:
> > + while (--count > 0)
>
> Shouldn't it be >= instead of > ?
>
Fixed.
Thanks,
-- Steve
> Thanks,
> Namhyung
>
>
> > + free(list[count]);
> > + free(list);
> > +
> > + return (char **)((unsigned long)-1);
> > +}
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
