From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757148AbcEEX6S (ORCPT ); Thu, 5 May 2016 19:58:18 -0400 Received: from mail.kernel.org ([198.145.29.136]:41441 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752786AbcEEX6R (ORCPT ); Thu, 5 May 2016 19:58:17 -0400 Date: Thu, 5 May 2016 20:58:12 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: linux-kernel@vger.kernel.org, Namhyung Kim , Peter Zijlstra , Ingo Molnar Subject: Re: [PATCH perf/core v2 7/8] perf help: Do not use ALLOC_GROW in add_cmd_list Message-ID: <20160505235812.GR11069@kernel.org> References: <20160429150941.30063.62888.stgit@devbox> <20160429151051.30063.35018.stgit@devbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160429151051.30063.35018.stgit@devbox> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Sat, Apr 30, 2016 at 12:10:51AM +0900, Masami Hiramatsu escreveu: > Replace ALLOC_GROW with normal realloc code in add_cmd_list() > so that it can handle errors directly. > > Signed-off-by: Masami Hiramatsu > --- > tools/perf/util/help-unknown-cmd.c | 30 ++++++++++++++++++++++-------- > 1 file changed, 22 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/util/help-unknown-cmd.c b/tools/perf/util/help-unknown-cmd.c > index 43a98a4..d62ccae 100644 > --- a/tools/perf/util/help-unknown-cmd.c > +++ b/tools/perf/util/help-unknown-cmd.c > @@ -27,16 +27,27 @@ static int levenshtein_compare(const void *p1, const void *p2) > return l1 != l2 ? l1 - l2 : strcmp(s1, s2); > } > > -static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) > +static int add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) > { > - unsigned int i; > - > - ALLOC_GROW(cmds->names, cmds->cnt + old->cnt, cmds->alloc); > - > + unsigned int i, nr = cmds->cnt + old->cnt; > + void *tmp; > + > + if (nr > cmds->alloc) { > + /* Choose bigger one to alloc */ > + if (alloc_nr(cmds->alloc) < nr) > + cmds->alloc = nr; > + else > + cmds->alloc = alloc_nr(cmds->alloc); > + tmp = realloc(cmds->names, cmds->alloc * sizeof(*cmds->names)); > + if (!tmp) > + return -1; Cool, here you use replace that big realloc-that-fails (ALLOC_GROW) but still uses realloc, checking its return, great. Looks ok. > + cmds->names = tmp; > + } > for (i = 0; i < old->cnt; i++) > cmds->names[cmds->cnt++] = old->names[i]; > zfree(&old->names); > old->cnt = 0; > + return 0; > } > > const char *help_unknown_cmd(const char *cmd) > @@ -52,8 +63,11 @@ const char *help_unknown_cmd(const char *cmd) > > load_command_list("perf-", &main_cmds, &other_cmds); > > - add_cmd_list(&main_cmds, &aliases); > - add_cmd_list(&main_cmds, &other_cmds); > + if (add_cmd_list(&main_cmds, &aliases) < 0 || > + add_cmd_list(&main_cmds, &other_cmds) < 0) { > + fprintf(stderr, "ERROR: Failed to allocate command list for unknown command.\n"); > + goto end; > + } > qsort(main_cmds.names, main_cmds.cnt, > sizeof(main_cmds.names), cmdname_compare); > uniq(&main_cmds); > @@ -99,6 +113,6 @@ const char *help_unknown_cmd(const char *cmd) > for (i = 0; i < n; i++) > fprintf(stderr, "\t%s\n", main_cmds.names[i]->name); > } > - > +end: > exit(1); > }