{"thread":{"id":"28435","subject":"[PATCH] git-mergetool--lib.sh: make check_unchanged return 1 on invalid read","startedAt":"2011-09-19T20:03:12Z","lastAt":"2011-09-20T01:20:22Z","messageCount":5,"participants":["Jay Soffian","Junio C Hamano","Andrew Ardill"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"175803","messageId":"1316462592-27255-1-git-send-email-jaysoffian@gmail.com","threadId":"28435","inReplyTo":null,"subject":"[PATCH] git-mergetool--lib.sh: make check_unchanged return 1 on invalid read","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-09-19T20:03:12Z","receivedAt":"2011-09-19T20:03:12Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Else when the user hits ctrl-c at the \"Was the merge successful?\n[y/n]\" prompt, mergetool goes into an infinite loop asking\nfor input.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\n git-mergetool--lib.sh |    6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 8fc65d0400..0eb424484c 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -21,7 +21,11 @@ check_unchanged () {\n \t\tdo\n \t\t\techo \"$MERGED seems unchanged.\"\n \t\t\tprintf \"Was the merge successful? [y/n] \"\n-\t\t\tread answer\n+\t\t\tif ! read answer\n+\t\t\tthen\n+\t\t\t\tstatus=1\n+\t\t\t\tbreak\n+\t\t\tfi\n \t\t\tcase \"$answer\" in\n \t\t\ty*|Y*) status=0; break ;;\n \t\t\tn*|N*) status=1; break ;;\n-- \n1.7.7.rc2.2.gf185\n"},{"id":"175808","messageId":"7vaaa09skn.fsf@alter.siamese.dyndns.org","threadId":"28435","inReplyTo":"1316462592-27255-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] git-mergetool--lib.sh: make check_unchanged return 1 on invalid read","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-19T20:37:44Z","receivedAt":"2011-09-19T20:37:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> Else when the user hits ctrl-c at the \"Was the merge successful?\n> [y/n]\" prompt, mergetool goes into an infinite loop asking\n> for input.\n>\n> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>\n\nWe still seem to miss one \"read\" unchecked in resolve_symlink_merge(),\neven with this patch.\n\n>  git-mergetool--lib.sh |    6 +++++-\n>  1 files changed, 5 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index 8fc65d0400..0eb424484c 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -21,7 +21,11 @@ check_unchanged () {\n>  \t\tdo\n>  \t\t\techo \"$MERGED seems unchanged.\"\n>  \t\t\tprintf \"Was the merge successful? [y/n] \"\n> -\t\t\tread answer\n> +\t\t\tif ! read answer\n> +\t\t\tthen\n> +\t\t\t\tstatus=1\n> +\t\t\t\tbreak\n> +\t\t\tfi\n\nI suspect that it would be more consistent with 6b44577 (mergetool: check\nreturn value from read, 2011-07-01), which this patch is a follow-up to,\nto do:\n\n\tread answer || return 1\n\nhere.\n\nThanks.\n"},{"id":"175825","messageId":"1316475652-35188-1-git-send-email-jaysoffian@gmail.com","threadId":"28435","inReplyTo":"7vaaa09skn.fsf@alter.siamese.dyndns.org","subject":"[PATCH] git-mergetool: check return value from read","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-09-19T23:40:52Z","receivedAt":"2011-09-19T23:40:52Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Mostly fixed already by 6b44577 (mergetool: check return value\nfrom read, 2011-07-01). Catch two uses it missed.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nOn Mon, Sep 19, 2011 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> We still seem to miss one \"read\" unchecked in resolve_symlink_merge(),\n> even with this patch.\n> [...]\n> I suspect that it would be more consistent with 6b44577 (mergetool: check\n> return value from read, 2011-07-01), which this patch is a follow-up to,\n> to do:\n>\n> \tread answer || return 1\n>\n> here.\n\nThanks, sorry I missed that.\n\n git-mergetool--lib.sh |    2 +-\n git-mergetool.sh      |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 8fc65d0400..ed630b208a 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -21,7 +21,7 @@ check_unchanged () {\n \t\tdo\n \t\t\techo \"$MERGED seems unchanged.\"\n \t\t\tprintf \"Was the merge successful? [y/n] \"\n-\t\t\tread answer\n+\t\t\tread answer || return 1\n \t\t\tcase \"$answer\" in\n \t\t\ty*|Y*) status=0; break ;;\n \t\t\tn*|N*) status=1; break ;;\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 3c157bcd26..b6d463f0d0 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -72,7 +72,7 @@ describe_file () {\n resolve_symlink_merge () {\n     while true; do\n \tprintf \"Use (l)ocal or (r)emote, or (a)bort? \"\n-\tread ans\n+\tread ans || return 1\n \tcase \"$ans\" in\n \t    [lL]*)\n \t\tgit checkout-index -f --stage=2 -- \"$MERGED\"\n-- \n1.7.7.rc2.2.gdf97720\n"},{"id":"175829","messageId":"7vboug82qk.fsf@alter.siamese.dyndns.org","threadId":"28435","inReplyTo":"1316475652-35188-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] git-mergetool: check return value from read","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-20T00:41:07Z","receivedAt":"2011-09-20T00:41:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n>> I suspect that it would be more consistent with 6b44577 (mergetool: check\n>> return value from read, 2011-07-01), which this patch is a follow-up to,\n>> to do:\n>>\n>> \tread answer || return 1\n>>\n>> here.\n>\n> Thanks, sorry I missed that.\n\nThank _you_ for spotting these unchecked \"read\"s.  Will queue.\n"},{"id":"175831","messageId":"CAH5451mt9mhRDQBYhUn=dO-SMyyhDNcv7nXfdsk-HKY3pMj77Q@mail.gmail.com","threadId":"28435","inReplyTo":"7vboug82qk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-mergetool: check return value from read","fromName":"Andrew Ardill","fromEmail":"andrew.ardill@gmail.com","sentAt":"2011-09-20T01:20:22Z","receivedAt":"2011-09-20T01:20:22Z","isPatch":true,"sender":{"key":"andrew.ardill@gmail.com","avatar":"https://gravatar.com/avatar/da14cb7c091dd44dc6c63a4d3361b149acaf25226dc78eb4131a17b93d9b0993?d=mp&s=160"},"body":"On 20 September 2011 10:41, Junio C Hamano <gitster@pobox.com> wrote:\n> Jay Soffian <jaysoffian@gmail.com> writes:\n>\n>>> I suspect that it would be more consistent with 6b44577 (mergetool: check\n>>> return value from read, 2011-07-01), which this patch is a follow-up to,\n>>> to do:\n>>>\n>>>      read answer || return 1\n>>>\n>>> here.\n>>\n>> Thanks, sorry I missed that.\n>\n> Thank _you_ for spotting these unchecked \"read\"s.  Will queue.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n\nI got hit by the ctrl+c bug while using mergetool just the other day.\nThanks for the fix :)\n"}]}