[Pdbg] [PATCH] libpdbg: Make pdbg_target_path() return a const string

Alistair Popple alistair at popple.id.au
Fri Jan 10 15:34:13 AEDT 2020


Currently pdbg_target_path() allocates memory to return the path
into. As the target path is a convient indentifier when printing
information to user about a specific target it leads either to memory
leaks when called directly as part of a printf statement. The
alternative is lots of repeated code like the below, noting that the
assert can never be hit as there is already one in dt_get_path():

path = pdbg_target_path(tgt);
assert(path);
printf("%s", path);
free(path);

Solve this by adding a cache of the path to struct pdbg_target and return
that instead.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 libpdbg/device.c  |  7 +++++--
 libpdbg/libpdbg.h |  2 +-
 libpdbg/target.h  |  1 +
 src/path.c        |  9 ++-------
 src/ring.c        |  7 +------
 src/scom.c        | 11 ++---------
 6 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/libpdbg/device.c b/libpdbg/device.c
index ad498dc..55d642e 100644
--- a/libpdbg/device.c
+++ b/libpdbg/device.c
@@ -774,9 +774,12 @@ bool pdbg_targets_init(void *fdt)
 	return true;
 }
 
-char *pdbg_target_path(struct pdbg_target *target)
+const char *pdbg_target_path(struct pdbg_target *target)
 {
-	return dt_get_path(target);
+	if (!target->path)
+		target->path = dt_get_path(target);
+
+	return target->path;
 }
 
 struct pdbg_target *pdbg_target_from_path(struct pdbg_target *target, const char *path)
diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
index 5816150..69fa2c7 100644
--- a/libpdbg/libpdbg.h
+++ b/libpdbg/libpdbg.h
@@ -115,7 +115,7 @@ enum pdbg_target_status pdbg_target_status(struct pdbg_target *target);
 void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status status);
 bool pdbg_set_backend(enum pdbg_backend backend, const char *backend_option);
 uint32_t pdbg_target_index(struct pdbg_target *target);
-char *pdbg_target_path(struct pdbg_target *target);
+const char *pdbg_target_path(struct pdbg_target *target);
 struct pdbg_target *pdbg_target_from_path(struct pdbg_target *target, const char *path);
 uint32_t pdbg_parent_index(struct pdbg_target *target, char *klass);
 const char *pdbg_target_class_name(struct pdbg_target *target);
diff --git a/libpdbg/target.h b/libpdbg/target.h
index a8f777d..b8e1d99 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -36,6 +36,7 @@ struct pdbg_target {
 	char *name;
 	char *compatible;
 	char *class;
+	const char* path;
 	int (*probe)(struct pdbg_target *target);
 	void (*release)(struct pdbg_target *target);
 	uint64_t (*translate)(struct pdbg_target *target, uint64_t addr);
diff --git a/src/path.c b/src/path.c
index a0eb666..b77ba79 100644
--- a/src/path.c
+++ b/src/path.c
@@ -339,14 +339,9 @@ bool path_target_all_selected(const char *classname, struct pdbg_target *parent)
 void path_target_dump(void)
 {
 	struct pdbg_target *target;
-	char *path;
 
-	for_each_path_target(target) {
-		path = pdbg_target_path(target);
-		assert(path);
-		printf("%s\n", path);
-		free(path);
-	}
+	for_each_path_target(target)
+		printf("%s\n", pdbg_target_path(target));
 }
 
 struct pdbg_target *path_target_next(struct pdbg_target *prev)
diff --git a/src/ring.c b/src/ring.c
index 1e40db6..adafac2 100644
--- a/src/ring.c
+++ b/src/ring.c
@@ -39,17 +39,12 @@ static int get_ring(uint64_t ring_addr, uint64_t ring_len)
 	assert(result);
 
 	for_each_path_target_class("chiplet", target) {
-		char *path;
 		int rc, i, len;
 
 		if (pdbg_target_status(target) != PDBG_TARGET_ENABLED)
 			continue;
 
-		path = pdbg_target_path(target);
-		assert(path);
-
-		printf("%s: 0x%016" PRIx64 " = ", path, ring_addr);
-		free(path);
+		printf("%s: 0x%016" PRIx64 " = ", pdbg_target_path(target), ring_addr);
 
 		rc = getring(target, ring_addr, ring_len, result);
 		if (rc) {
diff --git a/src/scom.c b/src/scom.c
index 6a59097..254c264 100644
--- a/src/scom.c
+++ b/src/scom.c
@@ -29,7 +29,7 @@
 int getscom(uint64_t addr)
 {
 	struct pdbg_target *target;
-	char *path;
+	const char *path;
 	uint64_t value;
 	int count = 0;
 
@@ -41,8 +41,6 @@ int getscom(uint64_t addr)
 			continue;
 
 		path = pdbg_target_path(target);
-		assert(path);
-
 		xlate_addr = addr;
 		addr_base = pdbg_address_absolute(target, &xlate_addr);
 		if (!addr_base) {
@@ -52,12 +50,10 @@ int getscom(uint64_t addr)
 
 		if (pib_read(target, addr, &value)) {
 			printf("p%d: 0x%016" PRIx64 " failed (%s)\n", pdbg_target_index(addr_base), xlate_addr, path);
-			free(path);
 			continue;
 		}
 
 		printf("p%d: 0x%016" PRIx64 " = 0x%016" PRIx64 " (%s)\n", pdbg_target_index(addr_base), xlate_addr, value, path);
-		free(path);
 		count++;
 	}
 
@@ -68,7 +64,7 @@ OPTCMD_DEFINE_CMD_WITH_ARGS(getscom, getscom, (ADDRESS));
 int putscom(uint64_t addr, uint64_t data, uint64_t mask)
 {
 	struct pdbg_target *target;
-	char *path;
+	const char *path;
 	int count = 0;
 
 	for_each_path_target(target) {
@@ -80,8 +76,6 @@ int putscom(uint64_t addr, uint64_t data, uint64_t mask)
 			continue;
 
 		path = pdbg_target_path(target);
-		assert(path);
-
 		xlate_addr = addr;
 		addr_base = pdbg_address_absolute(target, &xlate_addr);
 		if (!addr_base) {
@@ -96,7 +90,6 @@ int putscom(uint64_t addr, uint64_t data, uint64_t mask)
 
 		if (rc) {
 			printf("p%d: 0x%016" PRIx64 " failed (%s)\n", pdbg_target_index(addr_base), xlate_addr, path);
-			free(path);
 			continue;
 		}
 
-- 
2.20.1



More information about the Pdbg mailing list