Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756807AbbGQH4E (ORCPT ); Fri, 17 Jul 2015 03:56:04 -0400 Received: from mail-pd0-f177.google.com ([209.85.192.177]:36424 "EHLO mail-pd0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751343AbbGQH4B (ORCPT ); Fri, 17 Jul 2015 03:56:01 -0400 Date: Fri, 17 Jul 2015 16:42:05 +0900 From: Namhyung Kim To: Masami Hiramatsu Cc: Arnaldo Carvalho de Melo , Peter Zijlstra , linux-kernel@vger.kernel.org, Adrian Hunter , Ingo Molnar , Paul Mackerras , Jiri Olsa , Borislav Petkov , Hemant Kumar Subject: Re: [RFC PATCH perf/core v2 03/16] perf probe: Use strbuf for making strings in probe-event.c Message-ID: <20150717074205.GB25386@sejong> References: <20150715091352.8915.87480.stgit@localhost.localdomain> <20150715091414.8915.15879.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150715091414.8915.15879.stgit@localhost.localdomain> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5348 Lines: 199 Hi Masami, On Wed, Jul 15, 2015 at 06:14:14PM +0900, Masami Hiramatsu wrote: > Replace many fixed-length char array with strbuf to > stringify perf_probe_event and probe_trace_event etc. in > util/probe-event.c. > > Signed-off-by: Masami Hiramatsu > --- [SNIP] > + ret = strbuf_detach(&buf, NULL); > + strbuf_release(&buf); It seems that strbuf_release() is meaningless after strbuf_detach(). > > - return tmp - buf; > -error: > - pr_debug("Failed to synthesize perf probe argument: %d\n", ret); > return ret; > } > > /* Compose only probe point (not argument) */ > static char *synthesize_perf_probe_point(struct perf_probe_point *pp) > { > - char *buf, *tmp; > - char offs[32] = "", line[32] = "", file[32] = ""; > - int ret, len; > - > - buf = zalloc(MAX_CMDLEN); > - if (buf == NULL) { > - ret = -ENOMEM; > - goto error; > - } > - if (pp->offset) { > - ret = e_snprintf(offs, 32, "+%lu", pp->offset); > - if (ret <= 0) > - goto error; > - } > - if (pp->line) { > - ret = e_snprintf(line, 32, ":%d", pp->line); > - if (ret <= 0) > - goto error; > + struct strbuf buf; > + char *tmp; > + int len; > + > + strbuf_init(&buf, 64); > + if (pp->function) { > + strbuf_addstr(&buf, pp->function); > + if (pp->offset) > + strbuf_addf(&buf, "+%lu", pp->offset); > + else if (pp->line) > + strbuf_addf(&buf, ":%d", pp->line); > + else if (pp->retprobe) > + strbuf_addstr(&buf, "%return"); > } > if (pp->file) { > tmp = pp->file; > @@ -1633,25 +1616,14 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp) > tmp = strchr(pp->file + len - 30, '/'); > tmp = tmp ? tmp + 1 : pp->file + len - 30; > } > - ret = e_snprintf(file, 32, "@%s", tmp); > - if (ret <= 0) > - goto error; > + strbuf_addf(&buf, "@%s", tmp); > + if (!pp->function && pp->line) > + strbuf_addf(&buf, ":%d", pp->line); > } > > - if (pp->function) > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function, > - offs, pp->retprobe ? "%return" : "", line, > - file); > - else > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line); > - if (ret <= 0) > - goto error; > - > - return buf; > -error: > - pr_debug("Failed to synthesize perf probe point: %d\n", ret); > - free(buf); > - return NULL; > + tmp = strbuf_detach(&buf, NULL); > + strbuf_release(&buf); Ditto. > + return tmp; > } [SNIP] > char *synthesize_probe_trace_command(struct probe_trace_event *tev) > { > struct probe_trace_point *tp = &tev->point; > - char *buf; > - int i, len, ret; > - > - buf = zalloc(MAX_CMDLEN); > - if (buf == NULL) > - return NULL; > - > - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p', > - tev->group, tev->event); > - if (len <= 0) > - goto error; > + struct strbuf buf; > + char *ret = NULL; > + int i; > > /* Uprobes must have tp->address and tp->module */ > if (tev->uprobes && (!tp->address || !tp->module)) > - goto error; > + return NULL; > + > + strbuf_init(&buf, 32); > + strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p', > + tev->group, tev->event); > > /* Use the tp->address for uprobes */ > if (tev->uprobes) > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx", > - tp->module, tp->address); > + strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address); > else > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu", > - tp->module ?: "", tp->module ? ":" : "", > - tp->symbol, tp->offset); > - > - if (ret <= 0) > - goto error; > - len += ret; > + strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "", > + tp->module ? ":" : "", tp->symbol, tp->offset); > > for (i = 0; i < tev->nargs; i++) { > - ret = synthesize_probe_trace_arg(&tev->args[i], buf + len, > - MAX_CMDLEN - len); > - if (ret <= 0) > + if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0) > goto error; > - len += ret; > } > > - return buf; > + ret = strbuf_detach(&buf, NULL); > error: > - free(buf); > - return NULL; > + strbuf_release(&buf); This is necessary for the error path. > + return ret; > } > > static int find_perf_probe_point_from_map(struct probe_trace_point *tp, > @@ -1883,7 +1812,7 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp, > static int convert_to_perf_probe_event(struct probe_trace_event *tev, > struct perf_probe_event *pev, bool is_kprobe) > { > - char buf[64] = ""; > + struct strbuf buf = STRBUF_INIT; > int i, ret; > > /* Convert event/group name */ > @@ -1906,9 +1835,10 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev, > if (tev->args[i].name) > pev->args[i].name = strdup(tev->args[i].name); > else { > - ret = synthesize_probe_trace_arg(&tev->args[i], > - buf, 64); > - pev->args[i].name = strdup(buf); > + strbuf_init(&buf, 32); > + ret = synthesize_probe_trace_arg(&tev->args[i], &buf); > + pev->args[i].name = strbuf_detach(&buf, NULL); > + strbuf_release(&buf); Ditto. Thanks, Namhyung > } > if (pev->args[i].name == NULL && ret >= 0) > ret = -ENOMEM; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/