lkml.org 
[lkml]   [2015]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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;


\
 
 \ /
  Last update: 2015-07-17 10:01    [W:0.355 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site