{"thread":{"id":"47170","subject":"[PATCH] gpg-interface: Strip CR chars for Windows gpg2","startedAt":"2017-11-11T16:07:57Z","lastAt":"2017-11-12T05:58:27Z","messageCount":4,"participants":["Jerzy Kozera","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"332295","messageId":"20171111160657.11016-1-jerzy.kozera@gmail.com","threadId":"47170","inReplyTo":null,"subject":"[PATCH] gpg-interface: Strip CR chars for Windows gpg2","fromName":"Jerzy Kozera","fromEmail":"jerzy.kozera@gmail.com","sentAt":"2017-11-11T16:06:57Z","receivedAt":"2017-11-11T16:07:57Z","isPatch":true,"sender":{"key":"jerzy.kozera@gmail.com","avatar":"https://gravatar.com/avatar/f9b73794f2e389bd9b490213b1aced0cfced339c940e2f657ab43fedb21291e8?d=mp&s=160"},"body":"This fixes the issue with newlines reported at\nhttps://github.com/git-for-windows/git/issues/1249\n\nIssues with non-ASCII characters remain for further investigation.\n\nSigned-off-by: Jerzy Kozera \n---\nThe patch applies cleanly on top of maint as well as master.\n gpg-interface.c | 25 ++++++++++++++++---------\n 1 file changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 4feacf16e5..a92fb2f3b9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -145,6 +145,18 @@ const char *get_signing_key(void)\n \treturn git_committer_info(IDENT_STRICT|IDENT_NO_DATE);\n }\n \n+/* Strip CR from the line endings, in case we are on Windows. */\n+static void strip_cr(struct strbuf *buffer, size_t bottom) {\n+\tsize_t i, j;\n+\tfor (i = j = bottom; i < buffer->len; i++)\n+\t\tif (buffer->buf[i] != '\\r') {\n+\t\t\tif (i != j)\n+\t\t\t\tbuffer->buf[j] = buffer->buf[i];\n+\t\t\tj++;\n+\t\t}\n+\tstrbuf_setlen(buffer, j);\n+}\n+\n /*\n * Create a detached signature for the contents of \"buffer\" and append\n * it after \"signature\"; \"buffer\" and \"signature\" can be the same\n@@ -155,7 +167,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n-\tsize_t i, j, bottom;\n+\tsize_t bottom;\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \targv_array_pushl(&gpg.args,\n@@ -180,14 +192,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tif (ret)\n \t\treturn error(_(\"gpg failed to sign the data\"));\n \n-\t/* Strip CR from the line endings, in case we are on Windows. */\n-\tfor (i = j = bottom; i < signature->len; i++)\n-\t\tif (signature->buf[i] != '\\r') {\n-\t\t\tif (i != j)\n-\t\t\t\tsignature->buf[j] = signature->buf[i];\n-\t\t\tj++;\n-\t\t}\n-\tstrbuf_setlen(signature, j);\n+\tstrip_cr(signature, bottom);\n \n \treturn 0;\n }\n@@ -230,6 +235,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \tsigchain_push(SIGPIPE, SIG_IGN);\n \tret = pipe_command(&gpg, payload, payload_size,\n \t\t\t gpg_status, 0, gpg_output, 0);\n+\tstrip_cr(gpg_status, 0);\n+\tstrip_cr(gpg_output, 0);\n \tsigchain_pop(SIGPIPE);\n \n \tdelete_tempfile(&temp);\n-- \n2.14.2.windows.3\n\n"},{"id":"332305","messageId":"xmqqr2t4cli2.fsf@gitster.mtv.corp.google.com","threadId":"47170","inReplyTo":"20171111160657.11016-1-jerzy.kozera@gmail.com","subject":"Re: [PATCH] gpg-interface: Strip CR chars for Windows gpg2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-12T01:51:33Z","receivedAt":"2017-11-12T01:51:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jerzy Kozera writes:\n\n> +/* Strip CR from the line endings, in case we are on Windows. */\n> +static void strip_cr(struct strbuf *buffer, size_t bottom) {\n> +\tsize_t i, j;\n> +\tfor (i = j = bottom; i < buffer->len; i++)\n> +\t\tif (buffer->buf[i] != '\\r') {\n> +\t\t\tif (i != j)\n> +\t\t\t\tbuffer->buf[j] = buffer->buf[i];\n> +\t\t\tj++;\n> +\t\t}\n> +\tstrbuf_setlen(buffer, j);\n> +}\n> +\n\nWhile the approach of turning CRLF into LF as a workaround is a good\none, I do not see this loop doing that; instead it is removing CR\nanywhere on the line unconditionally. \n\nIt might not make a difference in practice to rely on the assumption\nthat nobody sane would place a lone CR in the middle of a line, but\nit feels dirty.\n\nBecause I know this was lifted from an existing code, I think it may\nbe better to take this patch as-is, but the code screams loudly that\nit wants a clean-up patch to fix that immediately on top of it.\n\nThanks.\n\n\n"},{"id":"332306","messageId":"xmqqmv3scley.fsf@gitster.mtv.corp.google.com","threadId":"47170","inReplyTo":"20171111160657.11016-1-jerzy.kozera@gmail.com","subject":"Re: [PATCH] gpg-interface: Strip CR chars for Windows gpg2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-12T01:53:25Z","receivedAt":"2017-11-12T01:53:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jerzy Kozera writes:\n\nOne minor detail I missed:\n\n> Subject: Re: [PATCH] gpg-interface: Strip CR chars for Windows gpg2\n\nPlease downcase \"Strip\" (see \"git shortlog --no-merges\" for recent\nhistory to notice that it would make this commit sit better with the\nexisting ones).\n"},{"id":"332310","messageId":"xmqqa7zsca2s.fsf@gitster.mtv.corp.google.com","threadId":"47170","inReplyTo":"20171111160657.11016-1-jerzy.kozera@gmail.com","subject":"Re: [PATCH] gpg-interface: Strip CR chars for Windows gpg2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-12T05:58:19Z","receivedAt":"2017-11-12T05:58:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jerzy Kozera writes:\n\n> This fixes the issue with newlines reported at\n> https://github.com/git-for-windows/git/issues/1249\n\nDo not omit the description of what you wanted to do completely and\nforce readers of \"git log\" to visit an external website. You are\nalready bothering to write \"issue with newlines\"; it shouldn't be\ntoo much to complete the description by saying what issues around\nnewlines you are solving, before saying \"reported at\".\n\nThanks.\n"}]}