On Tue, 24 Jul 2018 22:41:49 -0400 Steven Rostedt <[email protected]> wrote:
> On Wed, 25 Jul 2018 10:16:53 +0900 > Masami Hiramatsu <[email protected]> wrote: > > > Hm, as far as I can see, when register_trigger() returns >= 0, it already > > calls ->init the trigger_data. This means its refcount++, and in that > > case, below patch will miss to free the trigger_data. > > How about below for tentative fix? > > > > if (!ret) { > > ret = -ENOENT; > > /* We have to forcibly free the trigger_data */ > > goto out_free; > > } else if (ret > 0) > > ret = 0; > > Or better yet, match it properly: OK, this looks good to me :) Reviewed-by: Masami Hiramatsu <[email protected]> Thanks! > > out_reg: > /* Up the trigger_data count to make sure reg doesn't free it on > failure */ > event_trigger_init(trigger_ops, trigger_data); > ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file); > /* > * The above returns on success the # of functions enabled, > * but if it didn't find any functions it returns zero. > * Consider no functions a failure too. > */ > if (!ret) { > cmd_ops->unreg(glob, trigger_ops, trigger_data, file); > ret = -ENOENT; > } else if (ret > 0) > ret = 0; > > /* Down the counter of trigger_data or free it if not used anymore */ > event_trigger_free(trigger_ops, trigger_data); > out: > return ret; > > -- Steve -- Masami Hiramatsu <[email protected]>
