{"thread":{"id":"9142","subject":"Question about unpack-trees.c and tree_entry_list","startedAt":"2007-07-22T16:17:47Z","lastAt":"2007-07-24T21:54:25Z","messageCount":2,"participants":["David Kastrup","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"48148","messageId":"85d4ykjtuc.fsf@lola.goethe.zz","threadId":"9142","inReplyTo":null,"subject":"Question about unpack-trees.c and tree_entry_list","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-07-22T16:17:47Z","receivedAt":"2007-07-22T16:17:47Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\nThe definition of tree_entry_list ist the following\n\nstruct tree_entry_list {\n\tstruct tree_entry_list *next;\n\tunsigned directory : 1;\n\tunsigned executable : 1;\n\tunsigned symlink : 1;\n\tunsigned int mode;\n\tconst char *name;\n\tconst unsigned char *sha1;\n};\n\nbut it appears to me that the only of the bit fields that is used at\nall is \"directory\" : all other uses revert to \"mode\" which directory\npresumably could do as well.\n\nIs there something I am overlooking?\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"48525","messageId":"46A67511.8080300@lsrfire.ath.cx","threadId":"9142","inReplyTo":"85d4ykjtuc.fsf@lola.goethe.zz","subject":"[PATCH] cleanup unpack-trees.c: shrink struct tree_entry_list","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-07-24T21:54:25Z","receivedAt":"2007-07-24T21:54:25Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"David Kastrup schrieb:\n> The definition of tree_entry_list ist the following\n> \n> struct tree_entry_list {\n> \tstruct tree_entry_list *next;\n> \tunsigned directory : 1;\n> \tunsigned executable : 1;\n> \tunsigned symlink : 1;\n> \tunsigned int mode;\n> \tconst char *name;\n> \tconst unsigned char *sha1;\n> };\n> \n> but it appears to me that the only of the bit fields that is used at\n> all is \"directory\" : all other uses revert to \"mode\" which directory\n> presumably could do as well.\n> \n> Is there something I am overlooking?\n\nA C compiler can give the definite answer: no.\n\n--- 8< ---\nRemove the two write-only fields executable and symlink from struct\ntree_entry_list.  Also replace usage of the field directory with\nS_ISDIR checks on the mode field, and then remove this now obsolete\nfield, too.  Noticed by David Kastrup.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\nThis patch slightly reduces RAM usage, but it also shrinks the code\nwithout reducing functionality or readability, which is more\ninteresting.\n\n unpack-trees.c |   12 +++---------\n 1 files changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 7cc029e..3b32718 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -11,9 +11,6 @@\n \n struct tree_entry_list {\n \tstruct tree_entry_list *next;\n-\tunsigned directory : 1;\n-\tunsigned executable : 1;\n-\tunsigned symlink : 1;\n \tunsigned int mode;\n \tconst char *name;\n \tconst unsigned char *sha1;\n@@ -38,9 +35,6 @@ static struct tree_entry_list *create_tree_entry_list(struct tree *tree)\n \t\tentry->name = one.path;\n \t\tentry->sha1 = one.sha1;\n \t\tentry->mode = one.mode;\n-\t\tentry->directory = S_ISDIR(one.mode) != 0;\n-\t\tentry->executable = (one.mode & S_IXUSR) != 0;\n-\t\tentry->symlink = S_ISLNK(one.mode) != 0;\n \t\tentry->next = NULL;\n \n \t\t*list_p = entry;\n@@ -141,9 +135,9 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n #endif\n \t\t\tif (!first || entcmp(first, firstdir,\n \t\t\t\t\t     posns[i]->name,\n-\t\t\t\t\t     posns[i]->directory) > 0) {\n+\t\t\t\t\t     S_ISDIR(posns[i]->mode)) > 0) {\n \t\t\t\tfirst = posns[i]->name;\n-\t\t\t\tfirstdir = posns[i]->directory;\n+\t\t\t\tfirstdir = S_ISDIR(posns[i]->mode);\n \t\t\t}\n \t\t}\n \t\t/* No name means we're done */\n@@ -177,7 +171,7 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n \t\t\t\tcontinue;\n \t\t\t}\n \n-\t\t\tif (posns[i]->directory) {\n+\t\t\tif (S_ISDIR(posns[i]->mode)) {\n \t\t\t\tstruct tree *tree = lookup_tree(posns[i]->sha1);\n \t\t\t\tany_dirs = 1;\n \t\t\t\tparse_tree(tree);\n"}]}