Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752341AbbH1P5K (ORCPT ); Fri, 28 Aug 2015 11:57:10 -0400 Received: from mail.kernel.org ([198.145.29.136]:54377 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751693AbbH1P5I (ORCPT ); Fri, 28 Aug 2015 11:57:08 -0400 Date: Fri, 28 Aug 2015 12:56:45 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: Peter Zijlstra , linux-kernel@vger.kernel.org, Adrian Hunter , Ingo Molnar , Paul Mackerras , Jiri Olsa , Namhyung Kim , Borislav Petkov , Hemant Kumar Subject: Re: [RFC PATCH perf/core v3 01/17] perf probe: Use strbuf for making strings in probe-event.c Message-ID: <20150828155645.GA11407@kernel.org> References: <20150815114252.13642.62690.stgit@localhost.localdomain> <20150815114255.13642.16716.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150815114255.13642.16716.stgit@localhost.localdomain> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13120 Lines: 454 Em Sat, Aug 15, 2015 at 08:42:55PM +0900, Masami Hiramatsu escreveu: > Replace many fixed-length char array with strbuf to > stringify perf_probe_event and probe_trace_event etc. in > util/probe-event.c. Doesn't apply to perf/core. - Arnaldo > Signed-off-by: Masami Hiramatsu > --- > Changes in v3: > - Remove unneeded strbuf_release(). (Thanks Namhyung!) > > Changes in v2: > - Make perf_probe_event__sprintf() simpler. > --- > tools/perf/util/probe-event.c | 238 ++++++++++++++-------------------------- > tools/perf/util/probe-event.h | 3 - > tools/perf/util/probe-finder.c | 14 +- > 3 files changed, 90 insertions(+), 165 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index fe4941a..be417ee 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1562,69 +1562,51 @@ out: > } > > /* Compose only probe arg */ > -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len) > +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa) > { > struct perf_probe_arg_field *field = pa->field; > - int ret; > - char *tmp = buf; > + struct strbuf buf; > + char *ret; > > + strbuf_init(&buf, 64); > if (pa->name && pa->var) > - ret = e_snprintf(tmp, len, "%s=%s", pa->name, pa->var); > + strbuf_addf(&buf, "%s=%s", pa->name, pa->var); > else > - ret = e_snprintf(tmp, len, "%s", pa->name ? pa->name : pa->var); > - if (ret <= 0) > - goto error; > - tmp += ret; > - len -= ret; > + strbuf_addstr(&buf, pa->name ?: pa->var); > > while (field) { > if (field->name[0] == '[') > - ret = e_snprintf(tmp, len, "%s", field->name); > + strbuf_addstr(&buf, field->name); > else > - ret = e_snprintf(tmp, len, "%s%s", > - field->ref ? "->" : ".", field->name); > - if (ret <= 0) > - goto error; > - tmp += ret; > - len -= ret; > + strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".", > + field->name); > field = field->next; > } > > - if (pa->type) { > - ret = e_snprintf(tmp, len, ":%s", pa->type); > - if (ret <= 0) > - goto error; > - tmp += ret; > - len -= ret; > - } > + if (pa->type) > + strbuf_addf(&buf, ":%s", pa->type); > + > + ret = strbuf_detach(&buf, NULL); > > - 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 +1615,12 @@ 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; > + return strbuf_detach(&buf, NULL); > } > > #if 0 > @@ -1680,45 +1649,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev) > #endif > > static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref, > - char **buf, size_t *buflen, > - int depth) > + struct strbuf *buf, int depth) > { > - int ret; > if (ref->next) { > depth = __synthesize_probe_trace_arg_ref(ref->next, buf, > - buflen, depth + 1); > + depth + 1); > if (depth < 0) > goto out; > } > - > - ret = e_snprintf(*buf, *buflen, "%+ld(", ref->offset); > - if (ret < 0) > - depth = ret; > - else { > - *buf += ret; > - *buflen -= ret; > - } > + strbuf_addf(buf, "%+ld(", ref->offset); > out: > return depth; > - > } > > static int synthesize_probe_trace_arg(struct probe_trace_arg *arg, > - char *buf, size_t buflen) > + struct strbuf *buf) > { > struct probe_trace_arg_ref *ref = arg->ref; > - int ret, depth = 0; > - char *tmp = buf; > + int depth = 0; > > /* Argument name or separator */ > if (arg->name) > - ret = e_snprintf(buf, buflen, " %s=", arg->name); > + strbuf_addf(buf, " %s=", arg->name); > else > - ret = e_snprintf(buf, buflen, " "); > - if (ret < 0) > - return ret; > - buf += ret; > - buflen -= ret; > + strbuf_addch(buf, ' '); > > /* Special case: @XXX */ > if (arg->value[0] == '@' && arg->ref) > @@ -1726,86 +1680,58 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg, > > /* Dereferencing arguments */ > if (ref) { > - depth = __synthesize_probe_trace_arg_ref(ref, &buf, > - &buflen, 1); > + depth = __synthesize_probe_trace_arg_ref(ref, buf, 1); > if (depth < 0) > return depth; > } > > /* Print argument value */ > if (arg->value[0] == '@' && arg->ref) > - ret = e_snprintf(buf, buflen, "%s%+ld", arg->value, > - arg->ref->offset); > + strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset); > else > - ret = e_snprintf(buf, buflen, "%s", arg->value); > - if (ret < 0) > - return ret; > - buf += ret; > - buflen -= ret; > + strbuf_addstr(buf, arg->value); > > /* Closing */ > - while (depth--) { > - ret = e_snprintf(buf, buflen, ")"); > - if (ret < 0) > - return ret; > - buf += ret; > - buflen -= ret; > - } > + while (depth--) > + strbuf_addch(buf, ')'); > /* Print argument type */ > - if (arg->type) { > - ret = e_snprintf(buf, buflen, ":%s", arg->type); > - if (ret <= 0) > - return ret; > - buf += ret; > - } > + if (arg->type) > + strbuf_addf(buf, ":%s", arg->type); > > - return buf - tmp; > + return 0; > } > > 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); > + return ret; > } > > static int find_perf_probe_point_from_map(struct probe_trace_point *tp, > @@ -1883,7 +1809,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 +1832,9 @@ 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); > } > if (pev->args[i].name == NULL && ret >= 0) > ret = -ENOMEM; > @@ -2086,37 +2012,37 @@ static int perf_probe_event__sprintf(const char *group, const char *event, > const char *module, > struct strbuf *result) > { > - int i, ret; > - char buf[128]; > - char *place; > + int i; > + char *buf; > > - /* Synthesize only event probe point */ > - place = synthesize_perf_probe_point(&pev->point); > - if (!place) > - return -EINVAL; > + if (asprintf(&buf, "%s:%s", group, event) < 0) > + return -errno; > + strbuf_addf(result, " %-20s (on ", buf); > + free(buf); > > - ret = e_snprintf(buf, 128, "%s:%s", group, event); > - if (ret < 0) > - goto out; > + /* Synthesize only event probe point */ > + buf = synthesize_perf_probe_point(&pev->point); > + if (!buf) > + return -ENOMEM; > + strbuf_addstr(result, buf); > + free(buf); > > - strbuf_addf(result, " %-20s (on %s", buf, place); > if (module) > strbuf_addf(result, " in %s", module); > > if (pev->nargs > 0) { > strbuf_addstr(result, " with"); > for (i = 0; i < pev->nargs; i++) { > - ret = synthesize_perf_probe_arg(&pev->args[i], > - buf, 128); > - if (ret < 0) > - goto out; > + buf = synthesize_perf_probe_arg(&pev->args[i]); > + if (!buf) > + return -ENOMEM; > strbuf_addf(result, " %s", buf); > + free(buf); > } > } > strbuf_addch(result, ')'); > -out: > - free(place); > - return ret; > + > + return 0; > } > > /* Show an event */ > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > index 83ee95e..50216ff 100644 > --- a/tools/perf/util/probe-event.h > +++ b/tools/perf/util/probe-event.h > @@ -117,8 +117,7 @@ extern int parse_probe_trace_command(const char *cmd, > /* Events to command string */ > extern char *synthesize_perf_probe_command(struct perf_probe_event *pev); > extern char *synthesize_probe_trace_command(struct probe_trace_event *tev); > -extern int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, > - size_t len); > +extern char *synthesize_perf_probe_arg(struct perf_probe_arg *pa); > > /* Check the perf_probe_event needs debuginfo */ > extern bool perf_probe_event_need_dwarf(struct perf_probe_event *pev); > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c > index 7b80f8c..0c5168d 100644 > --- a/tools/perf/util/probe-finder.c > +++ b/tools/perf/util/probe-finder.c > @@ -550,7 +550,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf) > static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf) > { > Dwarf_Die vr_die; > - char buf[32], *ptr; > + char *buf, *ptr; > int ret = 0; > > if (!is_c_varname(pf->pvar->var)) { > @@ -575,13 +575,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf) > if (pf->pvar->name) > pf->tvar->name = strdup(pf->pvar->name); > else { > - ret = synthesize_perf_probe_arg(pf->pvar, buf, 32); > - if (ret < 0) > - return ret; > + buf = synthesize_perf_probe_arg(pf->pvar); > + if (!buf) > + return -ENOMEM; > ptr = strchr(buf, ':'); /* Change type separator to _ */ > if (ptr) > *ptr = '_'; > - pf->tvar->name = strdup(buf); > + pf->tvar->name = buf; > } > if (pf->tvar->name == NULL) > return -ENOMEM; > @@ -1318,8 +1318,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) > if (ret2 == 0) { > strlist__add(vl->vars, > strbuf_detach(&buf, NULL)); > - } > - strbuf_release(&buf); > + } else > + strbuf_release(&buf); > } > } > -- 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/