On 2015/07/17 16:42, Namhyung Kim wrote:
> 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...@hitachi.com>
>> ---
>
> [SNIP]
>
>> + ret = strbuf_detach(&buf, NULL);
>> + strbuf_release(&buf);
>
> It seems that strbuf_release() is meaningless after strbuf_detach().
Ah, right! I'll remove those redundant strbuf_release.
Thanks!
>
>
>>
>> - 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
