{"thread":{"id":"36040","subject":"[PATCH] cache_tree_find(): remove redundant check in while condition","startedAt":"2014-03-03T16:08:05Z","lastAt":"2014-03-03T17:25:52Z","messageCount":4,"participants":["Michael Haggerty","David Kastrup"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235882","messageId":"1393862885-23271-1-git-send-email-mhagger@alum.mit.edu","threadId":"36040","inReplyTo":null,"subject":"[PATCH] cache_tree_find(): remove redundant check in while condition","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-03T16:08:05Z","receivedAt":"2014-03-03T16:08:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\nA trivial little cleanup.\n\n cache-tree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 0bbec43..7f63c23 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -565,7 +565,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat\n \t\t\treturn NULL;\n \t\tit = sub->cache_tree;\n \t\tif (slash)\n-\t\t\twhile (*slash && *slash == '/')\n+\t\t\twhile (*slash == '/')\n \t\t\t\tslash++;\n \t\tif (!slash || !*slash)\n \t\t\treturn it; /* prefix ended with slashes */\n-- \n1.9.0\n"},{"id":"235883","messageId":"87a9d7kztm.fsf@fencepost.gnu.org","threadId":"36040","inReplyTo":"1393862885-23271-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH] cache_tree_find(): remove redundant check in while condition","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-03T16:32:37Z","receivedAt":"2014-03-03T16:32:37Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n> A trivial little cleanup.\n>\n>  cache-tree.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/cache-tree.c b/cache-tree.c\n> index 0bbec43..7f63c23 100644\n> --- a/cache-tree.c\n> +++ b/cache-tree.c\n> @@ -565,7 +565,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat\n>  \t\t\treturn NULL;\n>  \t\tit = sub->cache_tree;\n>  \t\tif (slash)\n> -\t\t\twhile (*slash && *slash == '/')\n> +\t\t\twhile (*slash == '/')\n>  \t\t\t\tslash++;\n>  \t\tif (!slash || !*slash)\n>  \t\t\treturn it; /* prefix ended with slashes */\n\nThat seems dragging around a NULL slash a lot.  How about not checking\nfor it multiple times?\n\n\t\tif (!slash)\n                \treturn it;\n\t\twhile (*slash == '/')\n\t\t\tslash++;\n\t\tif (!*slash)\n\t\t\treturn it; /* prefix ended with slashes */\n\nAs a bonus, the comment appears to be actually correct.  Attempting to\nverify its correctness by seeing whether a non-NULL slash is guaranteed\nto really end with slashes, we find the following code above for\ndefining slash:\n\n                slash = strchr(path, '/');\n                if (!slash)\n                        slash = path + strlen(path);\n\nSo it is literally impossible for slash to ever be NULL and all the\nchecking is nonsensical.  In addition, \"prefix ended with slashes\" does\nnot seem overly convincing when this code path is reached whether or not\nthere is a slash at all (I am not sure about it: it depends on the\npreceding find_subtree to some degree).\n\nSo perhaps all of that should just be\n\n\t\twhile (*slash == '/')\n\t\t\tslash++;\n\t\tif (!*slash)\n\t\t\treturn it;\n\n-- \nDavid Kastrup\n"},{"id":"235885","messageId":"5314B6E4.7030607@alum.mit.edu","threadId":"36040","inReplyTo":"87a9d7kztm.fsf@fencepost.gnu.org","subject":"Re: [PATCH] cache_tree_find(): remove redundant check in while condition","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-03T17:07:48Z","receivedAt":"2014-03-03T17:07:48Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/03/2014 05:32 PM, David Kastrup wrote:\n> [...]\n> So perhaps all of that should just be\n> \n> \t\twhile (*slash == '/')\n> \t\t\tslash++;\n> \t\tif (!*slash)\n> \t\t\treturn it;\n> \n\nI just fixed a little thing that caught my eye.  You OWNED it.  You are\nabsolutely right.\n\nWill you prepare a patch or would you like me to do it?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"235887","messageId":"8761nvkxcv.fsf@fencepost.gnu.org","threadId":"36040","inReplyTo":"5314B6E4.7030607@alum.mit.edu","subject":"Re: [PATCH] cache_tree_find(): remove redundant check in while condition","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-03T17:25:52Z","receivedAt":"2014-03-03T17:25:52Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> On 03/03/2014 05:32 PM, David Kastrup wrote:\n>> [...]\n>> So perhaps all of that should just be\n>> \n>> \t\twhile (*slash == '/')\n>> \t\t\tslash++;\n>> \t\tif (!*slash)\n>> \t\t\treturn it;\n>> \n>\n> I just fixed a little thing that caught my eye.  You OWNED it.  You are\n> absolutely right.\n>\n> Will you prepare a patch or would you like me to do it?\n\nFeel free.  I really should be finishing some other patches instead...\n\n-- \nDavid Kastrup\n"}]}