Messages in this thread | | | Date | Fri, 17 Jul 2015 16:42:05 +0900 | From | Namhyung Kim <> | Subject | Re: [RFC PATCH perf/core v2 03/16] perf probe: Use strbuf for making strings in probe-event.c |
| |
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 <masami.hiramatsu.pt@hitachi.com> > ---
[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;
| |