{"thread":{"id":"11877","subject":"[PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","startedAt":"2008-02-04T18:35:49Z","lastAt":"2008-02-05T20:38:40Z","messageCount":3,"participants":["Daniel Barkalow","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"67402","messageId":"alpine.LNX.1.00.0802041335470.13593@iabervon.org","threadId":"11877","inReplyTo":null,"subject":"[PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-04T18:35:49Z","receivedAt":"2008-02-04T18:35:49Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"Way back in read-tree.c, we used a mode 0 cache entry to indicate that\nan entry had been deleted, so that the update code would remove the\nworking tree file, and we would just skip it when writing out the\nindex file afterward.\n\nThese days, unpack_trees is a library function, and it is still\nleaving these entries in the active cache. Furthermore, unpack_trees\ndoesn't correctly ignore those entries, and who knows what other code\nwouldn't expect them to be there, but just isn't yet called after a\ncall to unpack_trees. To avoid having other code trip over these\nentries, have check_updates() remove them after it removes the working\ntree files.\n\nWhile we're at it, make the loop more obvious and skip passing in\nglobals to a static function with only one caller.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n unpack-trees.c |   16 +++++++++-------\n 1 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex f462a56..40d4130 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -288,16 +288,16 @@ static void unlink_entry(char *name, char *last_symlink)\n }\n \n static struct checkout state;\n-static void check_updates(struct cache_entry **src, int nr,\n-\t\t\tstruct unpack_trees_options *o)\n+static void check_updates(struct unpack_trees_options *o)\n {\n \tunsigned cnt = 0, total = 0;\n \tstruct progress *progress = NULL;\n \tchar last_symlink[PATH_MAX];\n+\tint i;\n \n \tif (o->update && o->verbose_update) {\n-\t\tfor (total = cnt = 0; cnt < nr; cnt++) {\n-\t\t\tstruct cache_entry *ce = src[cnt];\n+\t\tfor (total = cnt = 0; cnt < active_nr; cnt++) {\n+\t\t\tstruct cache_entry *ce = active_cache[cnt];\n \t\t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\t\ttotal++;\n \t\t}\n@@ -308,14 +308,16 @@ static void check_updates(struct cache_entry **src, int nr,\n \t}\n \n \t*last_symlink = '\\0';\n-\twhile (nr--) {\n-\t\tstruct cache_entry *ce = *src++;\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n \n \t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\tif (ce->ce_flags & CE_REMOVE) {\n \t\t\tif (o->update)\n \t\t\t\tunlink_entry(ce->name, last_symlink);\n+\t\t\tremove_cache_entry_at(i);\n+\t\t\ti--;\n \t\t\tcontinue;\n \t\t}\n \t\tif (ce->ce_flags & CE_UPDATE) {\n@@ -374,7 +376,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t\terror(\"Merge requires file-level merging\");\n \t}\n \n-\tcheck_updates(active_cache, active_nr, o);\n+\tcheck_updates(o);\n \treturn 0;\n }\n \n-- \n1.5.4\n"},{"id":"67465","messageId":"alpine.LSU.1.00.0802050123330.8543@racer.site","threadId":"11877","inReplyTo":"alpine.LNX.1.00.0802041335470.13593@iabervon.org","subject":"Re: [PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-05T01:25:52Z","receivedAt":"2008-02-05T01:25:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Feb 2008, Daniel Barkalow wrote:\n\n> Way back in read-tree.c, we used a mode 0 cache entry to indicate that \n> an entry had been deleted, so that the update code would remove the \n> working tree file, and we would just skip it when writing out the index \n> file afterward.\n> \n> These days, unpack_trees is a library function, and it is still leaving \n> these entries in the active cache. Furthermore, unpack_trees doesn't \n> correctly ignore those entries, and who knows what other code wouldn't \n> expect them to be there, but just isn't yet called after a call to \n> unpack_trees. To avoid having other code trip over these entries, have \n> check_updates() remove them after it removes the working tree files.\n> \n> While we're at it, make the loop more obvious and skip passing in \n> globals to a static function with only one caller.\n\nAfter reading the code I understand what you mean.  How about\n\n\tWhile at it, make the loop removing those entries more obvious, \n\tand avoid passing global variables as parameters to \n\tcheck_updates(): there is only one call site anyway.\n\nCiao,\nDscho\n"},{"id":"67553","messageId":"alpine.LNX.1.00.0802042122400.13593@iabervon.org","threadId":"11877","inReplyTo":"alpine.LSU.1.00.0802050123330.8543@racer.site","subject":"Re: [PATCH 4/9] Discard \"deleted\" cache entries after using them to update the working tree","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-05T20:38:40Z","receivedAt":"2008-02-05T20:38:40Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Mon, 4 Feb 2008, Daniel Barkalow wrote:\n> \n> > Way back in read-tree.c, we used a mode 0 cache entry to indicate that \n> > an entry had been deleted, so that the update code would remove the \n> > working tree file, and we would just skip it when writing out the index \n> > file afterward.\n> > \n> > These days, unpack_trees is a library function, and it is still leaving \n> > these entries in the active cache. Furthermore, unpack_trees doesn't \n> > correctly ignore those entries, and who knows what other code wouldn't \n> > expect them to be there, but just isn't yet called after a call to \n> > unpack_trees. To avoid having other code trip over these entries, have \n> > check_updates() remove them after it removes the working tree files.\n> > \n> > While we're at it, make the loop more obvious and skip passing in \n> > globals to a static function with only one caller.\n> \n> After reading the code I understand what you mean.  How about\n> \n> \tWhile at it, make the loop removing those entries more obvious, \n> \tand avoid passing global variables as parameters to \n> \tcheck_updates(): there is only one call site anyway.\n\nIt's the loop's flow control that I'm making more obvious (using a normal \nfor loop instead of a while loop that counts down the number of entries \nwhile counting up the pointer to the entry under consideration). I'll \nclarify the message for the next version.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}