# [PATCH] Small cache_tree_write refactor. 3 messages from 2007-09-25 to 2007-09-25. Participants: Pierre Habouzit, Johannes Schindelin. Thread: https://gitlist.dev/t/10009 ## Pierre Habouzit, 2007-09-25 08:22 Subject: [PATCH] Small cache_tree_write refactor. Message-ID: <20070925082341.DF412BDBCF@madism.org> URL: https://gitlist.dev/e/20070925082341.DF412BDBCF%40madism.org ``` 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 --- 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 ``` ## Johannes Schindelin, 2007-09-25 10:38 Subject: Re: [PATCH] Small cache_tree_write refactor. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0709251135530.28395%40racer.site In-Reply-To: <20070925082341.DF412BDBCF@madism.org> ``` 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 ``` ## Pierre Habouzit, 2007-09-25 11:40 Subject: Re: [PATCH] Small cache_tree_write refactor. Message-ID: <20070925114001.GB8308@artemis.corp> URL: https://gitlist.dev/e/20070925114001.GB8308%40artemis.corp In-Reply-To: ``` 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 ```