Message-Id: <20160510054707.6158.69861.stgit@devbox>
Date:	Tue, 10 May 2016 14:47:07 +0900
From:	Masami Hiramatsu <mhiramat@...nel.org>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	Masami Hiramatsu <mhiramat@...nel.org>,
	linux-kernel@...r.kernel.org, Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>
Subject: [PATCH perf/core v3 2/8] perf probe: Check the return value of strbuf APIs

Check the return value of strbuf APIs in perf-probe
related code, so that it can handle errors in strbuf.

Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
---
  Changes in v3:
   - Cleanup code path according to Arnaldo's review.
  Changes in V2:
   - Rebased on the latest perf/core.
---
 tools/perf/util/dwarf-aux.c    |   52 +++++++--------
 tools/perf/util/probe-event.c  |  143 ++++++++++++++++++++++++----------------
 tools/perf/util/probe-finder.c |   30 +++++---
 3 files changed, 128 insertions(+), 97 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index aea189b..a347b19 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -915,8 +915,7 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
 		tmp = "*";
 	else if (tag == DW_TAG_subroutine_type) {
 		/* Function pointer */
-		strbuf_add(buf, "(function_type)", 15);
-		return 0;
+		return strbuf_add(buf, "(function_type)", 15);
 	} else {
 		if (!dwarf_diename(&type))
 			return -ENOENT;
@@ -927,14 +926,10 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
 		else if (tag == DW_TAG_enumeration_type)
 			tmp = "enum ";
 		/* Write a base name */
-		strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type));
-		return 0;
+		return strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type));
 	}
 	ret = die_get_typename(&type, buf);
-	if (ret == 0)
-		strbuf_addstr(buf, tmp);
-
-	return ret;
+	return ret ? ret : strbuf_addstr(buf, tmp);
 }
 
 /**
@@ -951,12 +946,10 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
 	ret = die_get_typename(vr_die, buf);
 	if (ret < 0) {
 		pr_debug("Failed to get type, make it unknown.\n");
-		strbuf_add(buf, " (unknown_type)", 14);
+		ret = strbuf_add(buf, " (unknown_type)", 14);
 	}
 
-	strbuf_addf(buf, "\t%s", dwarf_diename(vr_die));
-
-	return 0;
+	return ret < 0 ? ret : strbuf_addf(buf, "\t%s", dwarf_diename(vr_die));
 }
 
 #ifdef HAVE_DWARF_GETLOCATIONS
@@ -999,22 +992,24 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
 	}
 
 	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
-				&start, &end)) > 0) {
+					&start, &end)) > 0) {
 		start -= entry;
 		end -= entry;
 
 		if (first) {
-			strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64,
-				name, start, end);
+			ret = strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64,
+					  name, start, end);
 			first = false;
 		} else {
-			strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64,
-				start, end);
+			ret = strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64,
+					  start, end);
 		}
+		if (ret < 0)
+			goto out;
 	}
 
 	if (!first)
-		strbuf_add(buf, "]>", 2);
+		ret = strbuf_add(buf, "]>", 2);
 
 out:
 	free(scopes);
@@ -1054,31 +1049,32 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
 	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
 		return -EINVAL;
 
-	while ((offset = dwarf_getlocations(
-				&attr, offset, &base,
-				&start, &end, &op, &nops)) > 0) {
+	while ((offset = dwarf_getlocations(&attr, offset, &base,
+					&start, &end, &op, &nops)) > 0) {
 		if (start == 0) {
 			/* Single Location Descriptions */
 			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
-			return ret;
+			goto out;
 		}
 
 		/* Location Lists */
 		start -= entry;
 		end -= entry;
 		if (first) {
-			strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64,
-				name, start, end);
+			ret = strbuf_addf(buf, "@<%s+[%" PRIu64 "-%" PRIu64,
+					  name, start, end);
 			first = false;
 		} else {
-			strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64,
-				start, end);
+			ret = strbuf_addf(buf, ",%" PRIu64 "-%" PRIu64,
+					  start, end);
 		}
+		if (ret < 0)
+			goto out;
 	}
 
 	if (!first)
-		strbuf_add(buf, "]>", 2);
-
+		ret = strbuf_add(buf, "]>", 2);
+out:
 	return ret;
 }
 #else
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c82c625..74401a2 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1677,28 +1677,37 @@ char *synthesize_perf_probe_arg(struct perf_probe_arg *pa)
 {
 	struct perf_probe_arg_field *field = pa->field;
 	struct strbuf buf;
-	char *ret;
+	char *ret = NULL;
+	int err;
+
+	if (strbuf_init(&buf, 64) < 0)
+		return NULL;
 
-	strbuf_init(&buf, 64);
 	if (pa->name && pa->var)
-		strbuf_addf(&buf, "%s=%s", pa->name, pa->var);
+		err = strbuf_addf(&buf, "%s=%s", pa->name, pa->var);
 	else
-		strbuf_addstr(&buf, pa->name ?: pa->var);
+		err = strbuf_addstr(&buf, pa->name ?: pa->var);
+	if (err)
+		goto out;
 
 	while (field) {
 		if (field->name[0] == '[')
-			strbuf_addstr(&buf, field->name);
+			err = strbuf_addstr(&buf, field->name);
 		else
-			strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".",
-				    field->name);
+			err = strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".",
+					  field->name);
 		field = field->next;
+		if (err)
+			goto out;
 	}
 
 	if (pa->type)
-		strbuf_addf(&buf, ":%s", pa->type);
+		if (strbuf_addf(&buf, ":%s", pa->type) < 0)
+			goto out;
 
 	ret = strbuf_detach(&buf, NULL);
-
+out:
+	strbuf_release(&buf);
 	return ret;
 }
 
@@ -1706,18 +1715,23 @@ char *synthesize_perf_probe_arg(struct perf_probe_arg *pa)
 static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
 {
 	struct strbuf buf;
-	char *tmp;
-	int len;
+	char *tmp, *ret = NULL;
+	int len, err = 0;
+
+	if (strbuf_init(&buf, 64) < 0)
+		return NULL;
 
-	strbuf_init(&buf, 64);
 	if (pp->function) {
-		strbuf_addstr(&buf, pp->function);
+		if (strbuf_addstr(&buf, pp->function) < 0)
+			goto out;
 		if (pp->offset)
-			strbuf_addf(&buf, "+%lu", pp->offset);
+			err = strbuf_addf(&buf, "+%lu", pp->offset);
 		else if (pp->line)
-			strbuf_addf(&buf, ":%d", pp->line);
+			err = strbuf_addf(&buf, ":%d", pp->line);
 		else if (pp->retprobe)
-			strbuf_addstr(&buf, "%return");
+			err = strbuf_addstr(&buf, "%return");
+		if (err)
+			goto out;
 	}
 	if (pp->file) {
 		tmp = pp->file;
@@ -1726,12 +1740,15 @@ 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;
 		}
-		strbuf_addf(&buf, "@%s", tmp);
-		if (!pp->function && pp->line)
-			strbuf_addf(&buf, ":%d", pp->line);
+		err = strbuf_addf(&buf, "@%s", tmp);
+		if (!err && !pp->function && pp->line)
+			err = strbuf_addf(&buf, ":%d", pp->line);
 	}
-
-	return strbuf_detach(&buf, NULL);
+	if (!err)
+		ret = strbuf_detach(&buf, NULL);
+out:
+	strbuf_release(&buf);
+	return ret;
 }
 
 #if 0
@@ -1762,28 +1779,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev)
 static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
 					    struct strbuf *buf, int depth)
 {
+	int err;
 	if (ref->next) {
 		depth = __synthesize_probe_trace_arg_ref(ref->next, buf,
 							 depth + 1);
 		if (depth < 0)
-			goto out;
+			return depth;
 	}
-	strbuf_addf(buf, "%+ld(", ref->offset);
-out:
-	return depth;
+	err = strbuf_addf(buf, "%+ld(", ref->offset);
+	return (err < 0) ? err : depth;
 }
 
 static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
 				      struct strbuf *buf)
 {
 	struct probe_trace_arg_ref *ref = arg->ref;
-	int depth = 0;
+	int depth = 0, err;
 
 	/* Argument name or separator */
 	if (arg->name)
-		strbuf_addf(buf, " %s=", arg->name);
+		err = strbuf_addf(buf, " %s=", arg->name);
 	else
-		strbuf_addch(buf, ' ');
+		err = strbuf_addch(buf, ' ');
+	if (err)
+		return err;
 
 	/* Special case: @XXX */
 	if (arg->value[0] == '@' && arg->ref)
@@ -1798,18 +1817,19 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
 
 	/* Print argument value */
 	if (arg->value[0] == '@' && arg->ref)
-		strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset);
+		err = strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset);
 	else
-		strbuf_addstr(buf, arg->value);
+		err = strbuf_addstr(buf, arg->value);
 
 	/* Closing */
-	while (depth--)
-		strbuf_addch(buf, ')');
+	while (!err && depth--)
+		err = strbuf_addch(buf, ')');
+
 	/* Print argument type */
-	if (arg->type)
-		strbuf_addf(buf, ":%s", arg->type);
+	if (!err && arg->type)
+		err = strbuf_addf(buf, ":%s", arg->type);
 
-	return 0;
+	return err;
 }
 
 char *synthesize_probe_trace_command(struct probe_trace_event *tev)
@@ -1817,15 +1837,18 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
 	struct probe_trace_point *tp = &tev->point;
 	struct strbuf buf;
 	char *ret = NULL;
-	int i;
+	int i, err;
 
 	/* Uprobes must have tp->module */
 	if (tev->uprobes && !tp->module)
 		return NULL;
 
-	strbuf_init(&buf, 32);
-	strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
-		    tev->group, tev->event);
+	if (strbuf_init(&buf, 32) < 0)
+		return NULL;
+
+	if (strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
+			tev->group, tev->event) < 0)
+		goto error;
 	/*
 	 * If tp->address == 0, then this point must be a
 	 * absolute address uprobe.
@@ -1839,14 +1862,16 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
 
 	/* Use the tp->address for uprobes */
 	if (tev->uprobes)
-		strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
+		err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
 	else if (!strncmp(tp->symbol, "0x", 2))
 		/* Absolute address. See try_to_find_absolute_address() */
-		strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
-			    tp->module ? ":" : "", tp->address);
+		err = strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
+				  tp->module ? ":" : "", tp->address);
 	else
-		strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
-			    tp->module ? ":" : "", tp->symbol, tp->offset);
+		err = strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
+				tp->module ? ":" : "", tp->symbol, tp->offset);
+	if (err)
+		goto error;
 
 	for (i = 0; i < tev->nargs; i++)
 		if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0)
@@ -1960,14 +1985,15 @@ 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 {
-			strbuf_init(&buf, 32);
+			if ((ret = strbuf_init(&buf, 32)) < 0)
+				goto error;
 			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;
 	}
-
+error:
 	if (ret < 0)
 		clear_perf_probe_event(pev);
 
@@ -2140,37 +2166,40 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
 				     const char *module,
 				     struct strbuf *result)
 {
-	int i;
+	int i, ret;
 	char *buf;
 
 	if (asprintf(&buf, "%s:%s", group, event) < 0)
 		return -errno;
-	strbuf_addf(result, "  %-20s (on ", buf);
+	ret = strbuf_addf(result, "  %-20s (on ", buf);
 	free(buf);
+	if (ret)
+		return ret;
 
 	/* Synthesize only event probe point */
 	buf = synthesize_perf_probe_point(&pev->point);
 	if (!buf)
 		return -ENOMEM;
-	strbuf_addstr(result, buf);
+	ret = strbuf_addstr(result, buf);
 	free(buf);
 
-	if (module)
-		strbuf_addf(result, " in %s", module);
+	if (!ret && module)
+		ret = strbuf_addf(result, " in %s", module);
 
-	if (pev->nargs > 0) {
-		strbuf_add(result, " with", 5);
-		for (i = 0; i < pev->nargs; i++) {
+	if (!ret && pev->nargs > 0) {
+		ret = strbuf_add(result, " with", 5);
+		for (i = 0; !ret && i < pev->nargs; i++) {
 			buf = synthesize_perf_probe_arg(&pev->args[i]);
 			if (!buf)
 				return -ENOMEM;
-			strbuf_addf(result, " %s", buf);
+			ret = strbuf_addf(result, " %s", buf);
 			free(buf);
 		}
 	}
-	strbuf_addch(result, ')');
+	if (!ret)
+		ret = strbuf_addch(result, ')');
 
-	return 0;
+	return ret;
 }
 
 /* Show an event */
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9f68875..1259839 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1294,6 +1294,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 {
 	struct available_var_finder *af = data;
 	struct variable_list *vl;
+	struct strbuf buf = STRBUF_INIT;
 	int tag, ret;
 
 	vl = &af->vls[af->nvls - 1];
@@ -1307,25 +1308,26 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 		if (ret == 0 || ret == -ERANGE) {
 			int ret2;
 			bool externs = !af->child;
-			struct strbuf buf;
 
-			strbuf_init(&buf, 64);
+			if (strbuf_init(&buf, 64) < 0)
+				goto error;
 
 			if (probe_conf.show_location_range) {
-				if (!externs) {
-					if (ret)
-						strbuf_add(&buf, "[INV]\t", 6);
-					else
-						strbuf_add(&buf, "[VAL]\t", 6);
-				} else
-					strbuf_add(&buf, "[EXT]\t", 6);
+				if (!externs)
+					ret2 = strbuf_add(&buf,
+						ret ? "[INV]\t" : "[VAL]\t", 6);
+				else
+					ret2 = strbuf_add(&buf, "[EXT]\t", 6);
+				if (ret2)
+					goto error;
 			}
 
 			ret2 = die_get_varname(die_mem, &buf);
 
 			if (!ret2 && probe_conf.show_location_range &&
 				!externs) {
-				strbuf_addch(&buf, '\t');
+				if (strbuf_addch(&buf, '\t') < 0)
+					goto error;
 				ret2 = die_get_var_range(&af->pf.sp_die,
 							die_mem, &buf);
 			}
@@ -1334,8 +1336,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 			if (ret2 == 0) {
 				strlist__add(vl->vars,
 					strbuf_detach(&buf, NULL));
-			} else
-				strbuf_release(&buf);
+			}
+			strbuf_release(&buf);
 		}
 	}
 
@@ -1343,6 +1345,10 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 		return DIE_FIND_CB_CONTINUE;
 	else
 		return DIE_FIND_CB_SIBLING;
+error:
+	strbuf_release(&buf);
+	pr_debug("Error in strbuf\n");
+	return DIE_FIND_CB_END;
 }
 
 /* Add a found vars into available variables list */

Powered by blists - more mailing lists