[PATCH] Small cache_tree_write refactor.

[PATCH] Small cache_tree_write refactor.

[Pierre Habouzit]

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 <madcoder@debian.org>
---
 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.

[Johannes Schindelin]

Hi,

On Tue, 25 Sep 2007, Pierre Habouzit wrote:

> --- 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.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

On Tue, Sep 25, 2007 at 10:38:16AM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 25 Sep 2007, Pierre Habouzit wrote:
> 
> > --- 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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]