[Skiboot] [PATCH v2] core/device: Remove redundant malloc

[Jordan Niethe]

Currently dt_new_addr() / dt_new_2addr() malloc a scatch buffer for the
node name. This is then duplicated in new_node(). This makes the initial
allocation redundant. To address this the allocation was removed from
dt_new_addr() / dt_new_2addr() and new_node() was changed to a varadic
function to accomodate the different name format.

Ideally vsnprintf() would be used to calculate the length of the
resulting name so a buffer could be dynamically allocated.
This would then be filled with another call to vsnprintf(). However the
implemenation of vsnprintf() does not behave correctly when called with
a size of 0 so this is not possible.
Instead a statically allocated buffer is used. This assumes a maximum
length for a name.
The optimization for ro memory names in take_name() is no longer
peformed. This is because the arguements to new_node() are intended for
vsnprintf() and can not be readly checked using is_rodata().
This does not significantly impact memory usage.

Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
...
It is possible to check read only memory by checking for a format string
indicator. This will now be performed.
---
 core/device.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/core/device.c b/core/device.c
index 6364a60e93ec..bfdfc7a22206 100644
--- a/core/device.c
+++ b/core/device.c
@@ -44,15 +44,31 @@ static void free_name(const char *name)
 		free((char *)name);
 }
 
-static struct dt_node *new_node(const char *name)
+static struct dt_node *new_node(const char *fmt, ...)
 {
 	struct dt_node *node = malloc(sizeof *node);
+	char name[256];
+	va_list args;
+
 	if (!node) {
 		prerror("Failed to allocate node\n");
 		abort();
 	}
 
-	node->name = take_name(name);
+	if (strchr(fmt, '%') || !is_rodata(fmt)) {
+		va_start(args, fmt);
+		vsnprintf(name, sizeof(name), fmt, args);
+		va_end(args);
+		node->name = strdup(name);
+	} else {
+		node->name = fmt;
+	}
+
+	if (!node->name) {
+		prerror("Failed to allocate copy of name");
+		abort();
+	}
+
 	node->parent = NULL;
 	list_head_init(&node->properties);
 	list_head_init(&node->children);
@@ -208,18 +224,10 @@ struct dt_node *dt_find_by_name_addr(struct dt_node *parent, const char *name,
 struct dt_node *dt_new_addr(struct dt_node *parent, const char *name,
 			    uint64_t addr)
 {
-	char *lname;
 	struct dt_node *new;
-	size_t len;
 
 	assert(parent);
-	len = strlen(name) + STR_MAX_CHARS(addr) + 2;
-	lname = malloc(len);
-	if (!lname)
-		return NULL;
-	snprintf(lname, len, "%s@%llx", name, (long long)addr);
-	new = new_node(lname);
-	free(lname);
+	new = new_node("%s@%llx", name, (long long)addr);
 	if (!dt_attach_root(parent, new)) {
 		dt_destroy(new);
 		return NULL;
@@ -230,19 +238,10 @@ struct dt_node *dt_new_addr(struct dt_node *parent, const char *name,
 struct dt_node *dt_new_2addr(struct dt_node *parent, const char *name,
 			     uint64_t addr0, uint64_t addr1)
 {
-	char *lname;
 	struct dt_node *new;
-	size_t len;
 	assert(parent);
 
-	len = strlen(name) + 2*STR_MAX_CHARS(addr0) + 3;
-	lname = malloc(len);
-	if (!lname)
-		return NULL;
-	snprintf(lname, len, "%s@%llx,%llx",
-		 name, (long long)addr0, (long long)addr1);
-	new = new_node(lname);
-	free(lname);
+	new = new_node("%s@%llx,%llx", name, (long long)addr0, (long long)addr1);
 	if (!dt_attach_root(parent, new)) {
 		dt_destroy(new);
 		return NULL;
-- 
2.20.1