On Thu, Apr 09, 2015 at 04:54:40PM +0200, Ingo Molnar wrote:
> > [...] If you unwind like:
> >
> > err_free_bar:
> > kfree(foo->bar);
> > err_free_foo:
> > kfree(foo);
> > return ret;
> >
> > That is less error prone.
>
> That's how I name and structure unwind labels as well, and my
> suggestion is to use something similar here in this code too, in
> arch/x86/kernel/cpu/perf_event_intel_pt.c.
>
> Agreed?
I don't understand what you want me to do here. I think you are saying
I should do this:
err_attrs:
kfree(attrs);
err:
return ret;
That's not the style that the rest of this file uses. Every function
uses direct returns where possible except pt_event_add() and that
function seems buggy.
arch/x86/kernel/cpu/perf_event_intel_pt.c
1000
1001 if (mode & PERF_EF_START) {
1002 pt_event_start(event, 0);
1003 if (hwc->state == PERF_HES_STOPPED) {
1004 pt_event_del(event, 0);
1005 ret = -EBUSY;
^^^^^^^^^^^^
We set "ret" here but then return zero.
1006 }
1007 } else {
1008 hwc->state = PERF_HES_STOPPED;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Shouldn't we set "ret" here?
1009 }
1010
1011 ret = 0;
1012 out:
1013
1014 if (ret)
1015 hwc->state = PERF_HES_STOPPED;
1016
1017 return ret;
1018 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
