{"thread":{"id":"28788","subject":"[PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","startedAt":"2011-10-28T15:28:28Z","lastAt":"2011-10-31T17:57:04Z","messageCount":8,"participants":["Sebastian Schuberth","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"178470","messageId":"4EAACA1C.6020302@gmail.com","threadId":"28788","inReplyTo":null,"subject":"[PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2011-10-28T15:28:28Z","receivedAt":"2011-10-28T15:28:28Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"For a plain string where only the length is known, strbuf.alloc needs to\nbe initialized to the length. Otherwise strbuf.alloc is 0 and a later\ncall to strbuf_setlen() will fail.\n\nThis bug surfaced when calling git blame under Windows on a *.doc file.\nThe *.doc file is converted to plain text by antiword via the textconv\nmechanism. However, the plain text returned by antiword contains DOS line\nendings instead of Unix line endings which triggered the strbuf_setlen()\nwhich previous to this patch failed.\n\nSigned-off-by: Sebastian Schuberth \n---\n builtin/blame.c | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 26a5d42..86c0537 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2113,8 +2113,10 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t\tswitch (st.st_mode & S_IFMT) {\n \t\tcase S_IFREG:\n \t\t\tif (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&\n-\t\t\t textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len))\n+\t\t\t textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len)) {\n+\t\t\t\tbuf.alloc = buf_len;\n \t\t\t\tbuf.len = buf_len;\n+\t\t\t}\n \t\t\telse if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n \t\t\t\tdie_errno(\"cannot open or read '%s'\", read_from);\n \t\t\tbreak;\n-- \n1.7.7.msysgit.1.4.gcc6bb.dirty\n"},{"id":"178473","messageId":"7vlis5t8bf.fsf@alter.siamese.dyndns.org","threadId":"28788","inReplyTo":"4EAACA1C.6020302@gmail.com","subject":"Re: [PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-28T16:00:36Z","receivedAt":"2011-10-28T16:00:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; do you have no addition to the test suite to demonstrate the\nbreakage?\n"},{"id":"178474","messageId":"CAHGBnuPUGfOe1D_OZ0ga4s8EiS_=GZeBG7WLmyOHt7vNg+w3Fw@mail.gmail.com","threadId":"28788","inReplyTo":"7vlis5t8bf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2011-10-28T16:32:15Z","receivedAt":"2011-10-28T16:32:15Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Fri, Oct 28, 2011 at 18:00, Junio C Hamano wrote:\n\n> Thanks; do you have no addition to the test suite to demonstrate the\n> breakage?\n\nNot yet. I'll try to come up with something.\n\n-- \nSebastian Schuberth\n"},{"id":"178475","messageId":"7vhb2tt6ag.fsf@alter.siamese.dyndns.org","threadId":"28788","inReplyTo":"CAHGBnuPUGfOe1D_OZ0ga4s8EiS_=GZeBG7WLmyOHt7vNg+w3Fw@mail.gmail.com","subject":"Re: [PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-28T16:44:23Z","receivedAt":"2011-10-28T16:44:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Schuberth writes:\n\n> On Fri, Oct 28, 2011 at 18:00, Junio C Hamano wrote:\n>\n>> Thanks; do you have no addition to the test suite to demonstrate the\n>> breakage?\n>\n> Not yet. I'll try to come up with something.\n\nLet's do this.\n\ndiff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh\nindex 32ec82a..4ee42f1 100755\n--- a/t/t8006-blame-textconv.sh\n+++ b/t/t8006-blame-textconv.sh\n@@ -15,6 +15,7 @@ EOF\n chmod +x helper\n \n test_expect_success 'setup ' '\n+\techo \"bin: test number 0\" >zero.bin &&\n \techo \"bin: test 1\" >one.bin &&\n \techo \"bin: test number 2\" >two.bin &&\n \tif test_have_prereq SYMLINKS; then\n@@ -43,6 +44,7 @@ test_expect_success 'no filter specified' '\n \n test_expect_success 'setup textconv filters' '\n \techo \"*.bin diff=test\" >.gitattributes &&\n+\techo \"zero.bin eol=crlf\" >>.gitattributes &&\n \tgit config diff.test.textconv ./helper &&\n \tgit config diff.test.cachetextconv false\n '\n@@ -74,6 +76,15 @@ test_expect_success 'blame --textconv going through revisions' '\n \ttest_cmp expected result\n '\n \n+test_expect_success 'blame --textconv with local changes' '\n+\ttest_when_finished \"git checkout zero.bin\" &&\n+\tprintf \"bin: updated number 0\\015\" >zero.bin &&\n+\tgit blame --textconv zero.bin >blame &&\n+\texpect=\"(Not Committed Yet ....-..-.. ..:..:.. +0000 1)\" &&\n+\texpect=\"$expect converted: updated number 0\" &&\n+\texpr \"$(find_blame wrote:\n\n>>> Thanks; do you have no addition to the test suite to demonstrate the\n>>> breakage?\n>>\n>> Not yet. I'll try to come up with something.\n>\n> Let's do this.\n\nThanks, but that does not seem to work for me. The test breaks both\nwithout and with my patch. I'll look into it.\n\n-- \nSebastian Schuberth\n"},{"id":"178477","messageId":"7vd3dht4ms.fsf@alter.siamese.dyndns.org","threadId":"28788","inReplyTo":"CAHGBnuNpwZCpQxZQNRZ-pYpLMHMi1O4d0hsR9MKM3=7Hw5A=zw@mail.gmail.com","subject":"Re: [PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-28T17:20:11Z","receivedAt":"2011-10-28T17:20:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Schuberth writes:\n\n> On Fri, Oct 28, 2011 at 18:44, Junio C Hamano wrote:\n>\n>>>> Thanks; do you have no addition to the test suite to demonstrate the\n>>>> breakage?\n>>>\n>>> Not yet. I'll try to come up with something.\n>>\n>> Let's do this.\n>\n> Thanks, but that does not seem to work for me. The test breaks both\n> without and with my patch. I'll look into it.\n\nThanks. I suspect the difference is because you are on a crlf-native\nplatform while I am not...\n"},{"id":"178483","messageId":"4EAAF63D.7050401@gmail.com","threadId":"28788","inReplyTo":"7vd3dht4ms.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2011-10-28T18:36:45Z","receivedAt":"2011-10-28T18:36:45Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 28.10.2011 19:20, Junio C Hamano wrote:\n\n>>>>> Thanks; do you have no addition to the test suite to demonstrate the\n>>>>> breakage?\n>>>>\n>>>> Not yet. I'll try to come up with something.\n>>>\n>>> Let's do this.\n>>\n>> Thanks, but that does not seem to work for me. The test breaks both\n>> without and with my patch. I'll look into it.\n> \n> Thanks. I suspect the difference is because you are on a crlf-native\n> platform while I am not...\n\nI also didn't have any luck. I've created a test that should fail without my patch, but it succeeds when running the test script. However, if I copy and paste the lines from the test to the command line, the test fails as expected (\"blame\" is empty). I'm out of ideas right now.\n\ndiff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh\nindex 32ec82a..4fee5aa 100755\n--- a/t/t8006-blame-textconv.sh\n+++ b/t/t8006-blame-textconv.sh\n@@ -14,6 +14,13 @@ sed 's/^bin: /converted: /' \"$1\"\n EOF\n chmod +x helper\n \n+cat >helper-dos-line-endings <<'EOF'\n+#!/bin/sh\n+grep -q '^bin: ' \"$1\" || { echo \"E: $1 is not \\\"binary\\\" file\" 1>&2; exit 1; }\n+sed 's/^bin: \\(.*\\)$/converted: \\1\\r/' \"$1\"\n+EOF\n+chmod +x helper-dos-line-endings\n+\n test_expect_success 'setup ' '\n \techo \"bin: test 1\" >one.bin &&\n \techo \"bin: test number 2\" >two.bin &&\n@@ -74,6 +81,14 @@ test_expect_success 'blame --textconv going through revisions' '\n \ttest_cmp expected result\n '\n \n+test_expect_success 'blame --textconv with DOS line endings' '\n+\tgit config diff.test.textconv ./helper-dos-line-endings &&\n+\tgit blame --textconv two.bin >blame &&\n+\tgit config diff.test.textconv ./helper &&\n+\tfind_blame result &&\n+\ttest_cmp expected result\n+'\n+\n test_expect_success 'setup +cachetextconv' '\n \tgit config diff.test.cachetextconv true\n '\n\n-- \nSebastian Schuberth\n"},{"id":"178567","messageId":"CAHGBnuPN9z_yntJPqGOWVFWc7PteWMfC8HV9UZoEYN-Z56xwyQ@mail.gmail.com","threadId":"28788","inReplyTo":"7vd3dht4ms.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] blame.c: Properly initialize strbuf after calling, textconv_object()","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2011-10-31T17:57:04Z","receivedAt":"2011-10-31T17:57:04Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Fri, Oct 28, 2011 at 19:20, Junio C Hamano wrote:\n\n>>>>> Thanks; do you have no addition to the test suite to demonstrate the\n>>>>> breakage?\n>>>>\n>>>> Not yet. I'll try to come up with something.\n>>>\n>>> Let's do this.\n>>\n>> Thanks, but that does not seem to work for me. The test breaks both\n>> without and with my patch. I'll look into it.\n>\n> Thanks. I suspect the difference is because you are on a crlf-native\n> platform while I am not...\n\nI've got a test now. However, that test revealed some more related\nissues. I'll come up with a v2 of the patch this week.\n\n-- \nSebastian Schuberth\n"}]}