On Mon, Apr 20, 2015 at 10:24:15AM +0200, Jiri Olsa wrote: > On Sun, Apr 19, 2015 at 01:04:13PM +0900, Namhyung Kim wrote: > > The options array saves strings for each popup menu item. The number of > > items can be vary according to the currently selected item. So it can > > leak some memory if it's exited from a small item. Fix it by freeing > > all items when loop terminates. > > > > Signed-off-by: Namhyung Kim <namhy...@kernel.org> > > --- > > tools/perf/ui/browsers/hists.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c > > index 8f303eb5044d..cace2df7e561 100644 > > --- a/tools/perf/ui/browsers/hists.c > > +++ b/tools/perf/ui/browsers/hists.c > > @@ -1691,7 +1691,8 @@ skip_annotation: > > "Switch to another data file in PWD") > 0) > > switch_data = nr_options++; > > add_exit_option: > > - options[nr_options++] = (char *)"Exit"; > > + if (asprintf(&options[nr_options], "Exit") > 0) > > + nr_options++; > > this one could cause segfault right? nice catch
Yes. But the original code don't cause segfault since it just stops at 'nr_options - 1'. When I changed it to go through the whole array, I found it caused a segfault, so changed it. Thanks, Namhyung > > > retry_popup_menu: > > choice = ui__popup_menu(nr_options, options); > > > > @@ -1812,7 +1813,7 @@ out_free_stack: > > pstack__delete(fstack); > > out: > > hist_browser__delete(browser); > > - free_popup_options(options, nr_options - 1); > > + free_popup_options(options, ARRAY_SIZE(options)); > > return key; > > } > > > > -- > > 2.3.5 > > -- 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/
