Re: [PATCH] perf_event: fix error handling path

[Corey Ashford]

On 12/07/2010 05:51 PM, jovi zhang wrote:
> On Mon, Dec 6, 2010 at 9:59 AM, jovi zhang<bookjovi@xxxxxxxxx>  wrote:
> > On Sun, Dec 5, 2010 at 8:29 PM, Thiago Farina<tfransosi@xxxxxxxxx>  wrote:
> > > On Sat, Dec 4, 2010 at 1:19 AM,<bookjovi@xxxxxxxxx>  wrote:
> > > > fix error handling path
> > > >
> > > > Signed-off-by: Jovi Zhang<bookjovi@xxxxxxxxx>
> > > >   kernel/perf_event.c |    2 --
> > > >   1 files changed, 0 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> > > > index cb6c0d2..62f9e9d 100644
> > > > --- a/kernel/perf_event.c
> > > > +++ b/kernel/perf_event.c
> > > > @@ -1918,8 +1918,6 @@ static int get_callchain_buffers(void)
> > > >         }
> > > >
> > > >         err = alloc_callchain_buffers();
> > > > -       if (err)
> > > > -               release_callchain_buffers();
> > >
> > > Care to explain in the change log message? As I reader, is not clear
> > > to me what is wrong with this.
> >
> > Sorry, the description should be as:
> > fix error handling path. alloc_callchain_buffers() can return -ENOMEM,
> > in this time callchain_cpus_entries maybe is NULL, It will oops if
> > invoke release_callchain_buffers() when callchain_cpus_entries is
> > NULL.
> I hope my understanding is right, is it?

One possible problem here is what if it returns an error other than 
-ENOMEM, and the buffers do need to be released?  Maybe you could change 
the code to

err = alloc_callchain_buffers();
if (err != -ENOMEM)
	release_callchain_buffers();


Currently, alloc_callchain_buffers cannot return any error code other 
than -ENOMEM, but that might change in the future.

- Corey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/