On Thu, 14 Apr 2016, Michael Ellerman wrote: > On Thu, 2016-04-14 at 14:01 +0200, Miroslav Benes wrote: > > On Wed, 13 Apr 2016, Michael Ellerman wrote: > > > > static void klp_disable_func(struct klp_func *func) > > > { > > > struct klp_ops *ops; > > > @@ -312,8 +325,14 @@ static void klp_disable_func(struct klp_func *func) > > > return; > > > > > > if (list_is_singular(&ops->func_stack)) { > > > + unsigned long ftrace_loc; > > > > This is a nit, but could you move the definition up to have them all in > > one place to be consistent with the rest of the code? The same applies to > > klp_enable_func() below. > > Hmm, actually I moved it in there because you pointed out we only needed it > inside the if: > > http://lkml.kernel.org/r/alpine.lnx.2.00.1603151113020.20...@pobox.suse.cz > > Thinking about it, we need ftrace_loc only in cases where we call > ftrace_set_filter_ip() right? So we can move klp_get_ftrace_location() > call to appropriate if branch both in klp_disable_func() and > klp_enable_func(). > > But I guess you meant the function call, not the variable declaration.
Exactly. > Personally I think it's better this way, as the variable is in scope for the > shortest possible amount of time, but I can change it if you want me to. No, it is nothing I would insist on. Thanks, Miroslav _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev