| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150717074205.GB25386@sejong>
Date: Fri, 17 Jul 2015 16:42:05 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
linux-kernel@...r.kernel.org,
Adrian Hunter <adrian.hunter@...el.com>,
Ingo Molnar <mingo@...hat.com>,
Paul Mackerras <paulus@...ba.org>,
Jiri Olsa <jolsa@...nel.org>, Borislav Petkov <bp@...e.de>,
Hemant Kumar <hemant@...ux.vnet.ibm.com>
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@...achi.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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists