[PATCH] Small cache_tree_write refactor.

Subsystems: the rest

DORMANTno replies

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] Small cache_tree_write refactor.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:36

This function cannot fail, make it void. Also make write_one act on a
const char* instead of a char*.

Signed-off-by: Pierre Habouzit <redacted>
---
 cache-tree.c |   19 +++++--------------
 cache-tree.h |    2 +-
 read-cache.c |   19 +++++++++----------
 3 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 5471844..50b3526 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -369,10 +369,8 @@ int cache_tree_update(struct cache_tree *it,
 	return 0;
 }
 
-static void write_one(struct cache_tree *it,
-		       char *path,
-		       int pathlen,
-			   struct strbuf *buffer)
+static void write_one(struct strbuf *buffer, struct cache_tree *it,
+                      const char *path, int pathlen)
 {
 	int i;
 
@@ -407,20 +405,13 @@ static void write_one(struct cache_tree *it,
 					     prev->name, prev->namelen) <= 0)
 				die("fatal - unsorted cache subtree");
 		}
-		write_one(down->cache_tree, down->name, down->namelen, buffer);
+		write_one(buffer, down->cache_tree, down->name, down->namelen);
 	}
 }
 
-void *cache_tree_write(struct cache_tree *root, unsigned long *size_p)
+void cache_tree_write(struct strbuf *sb, struct cache_tree *root)
 {
-	char path[PATH_MAX];
-	struct strbuf buffer;
-
-	path[0] = 0;
-	strbuf_init(&buffer, 0);
-	write_one(root, path, 0, &buffer);
-	*size_p = buffer.len;
-	return strbuf_detach(&buffer);
+	write_one(sb, root, "", 0);
 }
 
 static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
diff --git a/cache-tree.h b/cache-tree.h
index 119407e..8243228 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -22,7 +22,7 @@ void cache_tree_free(struct cache_tree **);
 void cache_tree_invalidate_path(struct cache_tree *, const char *);
 struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);
 
-void *cache_tree_write(struct cache_tree *root, unsigned long *size_p);
+void cache_tree_write(struct strbuf *, struct cache_tree *root);
 struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
 
 int cache_tree_fully_valid(struct cache_tree *);
diff --git a/read-cache.c b/read-cache.c
index 2e40a34..56202d1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1136,7 +1136,7 @@ int write_index(struct index_state *istate, int newfd)
 {
 	SHA_CTX c;
 	struct cache_header hdr;
-	int i, removed;
+	int i, err, removed;
 	struct cache_entry **cache = istate->cache;
 	int entries = istate->cache_nr;
 
@@ -1165,16 +1165,15 @@ int write_index(struct index_state *istate, int newfd)
 
 	/* Write extension data here */
 	if (istate->cache_tree) {
-		unsigned long sz;
-		void *data = cache_tree_write(istate->cache_tree, &sz);
-		if (data &&
-		    !write_index_ext_header(&c, newfd, CACHE_EXT_TREE, sz) &&
-		    !ce_write(&c, newfd, data, sz))
-			free(data);
-		else {
-			free(data);
+		struct strbuf sb;
+
+		strbuf_init(&sb, 0);
+		cache_tree_write(&sb, istate->cache_tree);
+		err = write_index_ext_header(&c, newfd, CACHE_EXT_TREE, sb.len) < 0
+			|| ce_write(&c, newfd, sb.buf, sb.len) < 0;
+		strbuf_release(&sb);
+		if (err)
 			return -1;
-		}
 	}
 	return ce_flush(&c, newfd);
 }
-- 
1.5.3.2.1067.g96579-dirty

Re: [PATCH] Small cache_tree_write refactor.

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:36

Hi,

On Tue, 25 Sep 2007, Pierre Habouzit wrote:
quoted hunk
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -369,10 +369,8 @@ int cache_tree_update(struct cache_tree *it,
 	return 0;
 }
 
-static void write_one(struct cache_tree *it,
-		       char *path,
-		       int pathlen,
-			   struct strbuf *buffer)
+static void write_one(struct strbuf *buffer, struct cache_tree *it,
+                      const char *path, int pathlen)
I don't know... is this really needed?  In some other projects, the coding 
standard prefers the parameters in "in"..."out" order.

Ciao,
Dscho

Re: [PATCH] Small cache_tree_write refactor.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:36

On Tue, Sep 25, 2007 at 10:38:16AM +0000, Johannes Schindelin wrote:
Hi,

On Tue, 25 Sep 2007, Pierre Habouzit wrote:
quoted
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -369,10 +369,8 @@ int cache_tree_update(struct cache_tree *it,
 	return 0;
 }
 
-static void write_one(struct cache_tree *it,
-		       char *path,
-		       int pathlen,
-			   struct strbuf *buffer)
+static void write_one(struct strbuf *buffer, struct cache_tree *it,
+                      const char *path, int pathlen)
I don't know... is this really needed?  In some other projects, the coding 
standard prefers the parameters in "in"..."out" order.
  Well, this is thought in an OO way, buffer would be the "this". This
method could be named strbuf_addtree(...) hence I felt that having the
buffer as a first argument to be right.

  But I don't care that much about that.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help