{"thread":{"id":"52028","subject":"[PATCH] http-push: simplify deleting a list item","startedAt":"2019-10-13T12:49:50Z","lastAt":"2019-10-15T01:53:51Z","messageCount":2,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"383992","messageId":"3739b392-ee45-98c2-c5fa-e4c36e585166@web.de","threadId":"52028","inReplyTo":null,"subject":"[PATCH] http-push: simplify deleting a list item","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-10-13T12:49:17Z","receivedAt":"2019-10-13T12:49:50Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The first step for deleting an item from a linked list is to locate the\nitem preceding it.  Be more careful in release_request() and handle an\nempty list.  This only has consequences for invalid delete requests\n(removing the same item twice, or deleting an item that was never added\nto the list), but simplifies the loop condition as well as the check\nafter the loop.\n\nOnce we found the item's predecessor in the list, update its next\npointer to skip over the item, which removes it from the list.  In other\nwords: Make the item's successor the successor of its predecessor.\n(At this point entry->next == request and prev->next == lock,\nrespectively.)  This is a bit simpler and saves a pointer dereference.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n http-push.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 0353f9f514..822f326599 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -501,10 +501,10 @@ static void release_request(struct transfer_request *request)\n \tif (request == request_queue_head) {\n \t\trequest_queue_head = request->next;\n \t} else {\n-\t\twhile (entry->next != NULL && entry->next != request)\n+\t\twhile (entry && entry->next != request)\n \t\t\tentry = entry->next;\n-\t\tif (entry->next == request)\n-\t\t\tentry->next = entry->next->next;\n+\t\tif (entry)\n+\t\t\tentry->next = request->next;\n \t}\n\n \tfree(request->url);\n@@ -981,7 +981,7 @@ static int unlock_remote(struct remote_lock *lock)\n \t\twhile (prev && prev->next != lock)\n \t\t\tprev = prev->next;\n \t\tif (prev)\n-\t\t\tprev->next = prev->next->next;\n+\t\t\tprev->next = lock->next;\n \t}\n\n \tfree(lock->owner);\n--\n2.23.0\n"},{"id":"384055","messageId":"xmqqtv8adba1.fsf@gitster-ct.c.googlers.com","threadId":"52028","inReplyTo":"3739b392-ee45-98c2-c5fa-e4c36e585166@web.de","subject":"Re: [PATCH] http-push: simplify deleting a list item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-15T01:53:42Z","receivedAt":"2019-10-15T01:53:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> The first step for deleting an item from a linked list is to locate the\n> item preceding it.  Be more careful in release_request() and handle an\n> empty list.  This only has consequences for invalid delete requests\n> (removing the same item twice, or deleting an item that was never added\n> to the list), but simplifies the loop condition as well as the check\n> after the loop.\n>\n> Once we found the item's predecessor in the list, update its next\n> pointer to skip over the item, which removes it from the list.  In other\n> words: Make the item's successor the successor of its predecessor.\n> (At this point entry->next == request and prev->next == lock,\n> respectively.)  This is a bit simpler and saves a pointer dereference.\n>\n> Signed-off-by: René Scharfe <l.s.r@web.de>\n> ---\n>  http-push.c | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n\nNice simplification.  I wonder how much longer we should be\nmaintaining this program, though;-)\n\n\nWill queue.\n\n>\n> diff --git a/http-push.c b/http-push.c\n> index 0353f9f514..822f326599 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -501,10 +501,10 @@ static void release_request(struct transfer_request *request)\n>  \tif (request == request_queue_head) {\n>  \t\trequest_queue_head = request->next;\n>  \t} else {\n> -\t\twhile (entry->next != NULL && entry->next != request)\n> +\t\twhile (entry && entry->next != request)\n>  \t\t\tentry = entry->next;\n> -\t\tif (entry->next == request)\n> -\t\t\tentry->next = entry->next->next;\n> +\t\tif (entry)\n> +\t\t\tentry->next = request->next;\n>  \t}\n>\n>  \tfree(request->url);\n> @@ -981,7 +981,7 @@ static int unlock_remote(struct remote_lock *lock)\n>  \t\twhile (prev && prev->next != lock)\n>  \t\t\tprev = prev->next;\n>  \t\tif (prev)\n> -\t\t\tprev->next = prev->next->next;\n> +\t\t\tprev->next = lock->next;\n>  \t}\n>\n>  \tfree(lock->owner);\n> --\n> 2.23.0\n"}]}