{"thread":{"id":"10009","subject":"[PATCH] Small cache_tree_write refactor.","startedAt":"2007-09-25T08:22:44Z","lastAt":"2007-09-25T11:40:01Z","messageCount":3,"participants":["Pierre Habouzit","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"53971","messageId":"20070925082341.DF412BDBCF@madism.org","threadId":"10009","inReplyTo":null,"subject":"[PATCH] Small cache_tree_write refactor.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-25T08:22:44Z","receivedAt":"2007-09-25T08:22:44Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"This function cannot fail, make it void. Also make write_one act on a\nconst char* instead of a char*.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n cache-tree.c |   19 +++++--------------\n cache-tree.h |    2 +-\n read-cache.c |   19 +++++++++----------\n 3 files changed, 15 insertions(+), 25 deletions(-)\n\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 5471844..50b3526 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -369,10 +369,8 @@ int cache_tree_update(struct cache_tree *it,\n \treturn 0;\n }\n \n-static void write_one(struct cache_tree *it,\n-\t\t       char *path,\n-\t\t       int pathlen,\n-\t\t\t   struct strbuf *buffer)\n+static void write_one(struct strbuf *buffer, struct cache_tree *it,\n+                      const char *path, int pathlen)\n {\n \tint i;\n \n@@ -407,20 +405,13 @@ static void write_one(struct cache_tree *it,\n \t\t\t\t\t     prev->name, prev->namelen) <= 0)\n \t\t\t\tdie(\"fatal - unsorted cache subtree\");\n \t\t}\n-\t\twrite_one(down->cache_tree, down->name, down->namelen, buffer);\n+\t\twrite_one(buffer, down->cache_tree, down->name, down->namelen);\n \t}\n }\n \n-void *cache_tree_write(struct cache_tree *root, unsigned long *size_p)\n+void cache_tree_write(struct strbuf *sb, struct cache_tree *root)\n {\n-\tchar path[PATH_MAX];\n-\tstruct strbuf buffer;\n-\n-\tpath[0] = 0;\n-\tstrbuf_init(&buffer, 0);\n-\twrite_one(root, path, 0, &buffer);\n-\t*size_p = buffer.len;\n-\treturn strbuf_detach(&buffer);\n+\twrite_one(sb, root, \"\", 0);\n }\n \n static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)\ndiff --git a/cache-tree.h b/cache-tree.h\nindex 119407e..8243228 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -22,7 +22,7 @@ void cache_tree_free(struct cache_tree **);\n void cache_tree_invalidate_path(struct cache_tree *, const char *);\n struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);\n \n-void *cache_tree_write(struct cache_tree *root, unsigned long *size_p);\n+void cache_tree_write(struct strbuf *, struct cache_tree *root);\n struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);\n \n int cache_tree_fully_valid(struct cache_tree *);\ndiff --git a/read-cache.c b/read-cache.c\nindex 2e40a34..56202d1 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1136,7 +1136,7 @@ int write_index(struct index_state *istate, int newfd)\n {\n \tSHA_CTX c;\n \tstruct cache_header hdr;\n-\tint i, removed;\n+\tint i, err, removed;\n \tstruct cache_entry **cache = istate->cache;\n \tint entries = istate->cache_nr;\n \n@@ -1165,16 +1165,15 @@ int write_index(struct index_state *istate, int newfd)\n \n \t/* Write extension data here */\n \tif (istate->cache_tree) {\n-\t\tunsigned long sz;\n-\t\tvoid *data = cache_tree_write(istate->cache_tree, &sz);\n-\t\tif (data &&\n-\t\t    !write_index_ext_header(&c, newfd, CACHE_EXT_TREE, sz) &&\n-\t\t    !ce_write(&c, newfd, data, sz))\n-\t\t\tfree(data);\n-\t\telse {\n-\t\t\tfree(data);\n+\t\tstruct strbuf sb;\n+\n+\t\tstrbuf_init(&sb, 0);\n+\t\tcache_tree_write(&sb, istate->cache_tree);\n+\t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_TREE, sb.len) < 0\n+\t\t\t|| ce_write(&c, newfd, sb.buf, sb.len) < 0;\n+\t\tstrbuf_release(&sb);\n+\t\tif (err)\n \t\t\treturn -1;\n-\t\t}\n \t}\n \treturn ce_flush(&c, newfd);\n }\n-- \n1.5.3.2.1067.g96579-dirty\n"},{"id":"53981","messageId":"Pine.LNX.4.64.0709251135530.28395@racer.site","threadId":"10009","inReplyTo":"20070925082341.DF412BDBCF@madism.org","subject":"Re: [PATCH] Small cache_tree_write refactor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T10:38:16Z","receivedAt":"2007-09-25T10:38:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Sep 2007, Pierre Habouzit wrote:\n\n> --- a/cache-tree.c\n> +++ b/cache-tree.c\n> @@ -369,10 +369,8 @@ int cache_tree_update(struct cache_tree *it,\n>  \treturn 0;\n>  }\n>  \n> -static void write_one(struct cache_tree *it,\n> -\t\t       char *path,\n> -\t\t       int pathlen,\n> -\t\t\t   struct strbuf *buffer)\n> +static void write_one(struct strbuf *buffer, struct cache_tree *it,\n> +                      const char *path, int pathlen)\n\nI don't know... is this really needed?  In some other projects, the coding \nstandard prefers the parameters in \"in\"...\"out\" order.\n\nCiao,\nDscho\n"},{"id":"53984","messageId":"20070925114001.GB8308@artemis.corp","threadId":"10009","inReplyTo":"Pine.LNX.4.64.0709251135530.28395@racer.site","subject":"Re: [PATCH] Small cache_tree_write refactor.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-25T11:40:01Z","receivedAt":"2007-09-25T11:40:01Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Sep 25, 2007 at 10:38:16AM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 25 Sep 2007, Pierre Habouzit wrote:\n> \n> > --- a/cache-tree.c\n> > +++ b/cache-tree.c\n> > @@ -369,10 +369,8 @@ int cache_tree_update(struct cache_tree *it,\n> >  \treturn 0;\n> >  }\n> >  \n> > -static void write_one(struct cache_tree *it,\n> > -\t\t       char *path,\n> > -\t\t       int pathlen,\n> > -\t\t\t   struct strbuf *buffer)\n> > +static void write_one(struct strbuf *buffer, struct cache_tree *it,\n> > +                      const char *path, int pathlen)\n> \n> I don't know... is this really needed?  In some other projects, the coding \n> standard prefers the parameters in \"in\"...\"out\" order.\n\n  Well, this is thought in an OO way, buffer would be the \"this\". This\nmethod could be named strbuf_addtree(...) hence I felt that having the\nbuffer as a first argument to be right.\n\n  But I don't care that much about that.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"}]}