{"thread":{"id":"34461","subject":"[PATCH] TIG: Fix to reinstate proper operation with no arguments","startedAt":"2013-07-18T04:51:53Z","lastAt":"2013-07-29T01:23:00Z","messageCount":7,"participants":["Drew Northup","Jonas Fonseca"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"223635","messageId":"1374123113-3889-1-git-send-email-n1xim.email@gmail.com","threadId":"34461","inReplyTo":null,"subject":"[PATCH] TIG: Fix to reinstate proper operation with no arguments","fromName":"Drew Northup","fromEmail":"n1xim.email@gmail.com","sentAt":"2013-07-18T04:51:53Z","receivedAt":"2013-07-18T04:51:53Z","isPatch":true,"sender":{"key":"n1xim.email@gmail.com","avatar":null},"body":"Since c7d67ab running \"tig\" with no options has failed with the\nerror \"tig: No revisions match the given arguments.\" This was due\nto a change in how the arguments for the back-end git call was\nbeing constructed. This change caused the blank field left in\nplace of \"(encoding_arg)\" when it is empty to not overwrite\n\"buf\" which then caused the value in \"buf\" to be copied into\ndst_argv twice. The resulting git command failed if there was no\navailable revision named \"log\" as shown in the trace.\n\n>From the TIG_TRACE log:\ngit log log --no-color --pretty=raw --parents --parents --\nfatal: bad revision 'log'\n\nThis fix works by teaching tig that when it is supplied with a\nblank field in the source argument buffer that it should skip\nover that field and continue instead of copying the previous\nfield value into the destination buffer a second time.\n\ngithub issue # 167\n\nSigned-off-by: Drew Northup \n---\n\nThis should apply cleanly to the tig public master whether the\nmkstemps() patch I wrote has been applied or not.\n\n tig.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/tig.c b/tig.c\nindex ba9ba98..1016cfe 100644\n--- a/tig.c\n+++ b/tig.c\n@@ -3105,10 +3105,11 @@ static bool\n format_append_arg(struct format_context *format, const char ***dst_argv, const char *arg)\n {\n \tformat->bufpos = 0;\n+\tint len = 0;\n \n \twhile (arg) {\n \t\tchar *next = strstr(arg, \"%(\");\n-\t\tint len = next ? next - arg : strlen(arg);\n+\t\tlen = next ? next - arg : strlen(arg);\n \n \t\tif (len && !string_format_from(format->buf, &format->bufpos, \"%.*s\", len, arg))\n \t\t\treturn FALSE;\n@@ -3119,7 +3120,11 @@ format_append_arg(struct format_context *format, const char ***dst_argv, const c\n \t\targ = next ? strchr(next, ')') + 1 : NULL;\n \t}\n \n-\treturn argv_append(dst_argv, format->buf);\n+\tif(len){\n+\t\treturn argv_append(dst_argv, format->buf);\n+\t} else {\n+\t\treturn TRUE;\n+\t}\n }\n \n static bool\n-- \n1.8.0\n"},{"id":"223647","messageId":"51E7EDE3.4080205@gmail.com","threadId":"34461","inReplyTo":"1374123113-3889-1-git-send-email-n1xim.email@gmail.com","subject":"Re: [PATCH] TIG: Fix to reinstate proper operation with no arguments","fromName":"Drew Northup","fromEmail":"n1xim.email@gmail.com","sentAt":"2013-07-18T13:30:11Z","receivedAt":"2013-07-18T13:30:11Z","isPatch":true,"sender":{"key":"n1xim.email@gmail.com","avatar":null},"body":"Somehow this patch breaks the main view to not open the correct commit \nin diff view when is pressed. Back to the debugger...\n\nOn 07/18/2013 12:51 AM, Drew Northup wrote:\n> Since c7d67ab running \"tig\" with no options has failed with the\n> error \"tig: No revisions match the given arguments.\" This was due\n> to a change in how the arguments for the back-end git call was\n> being constructed. This change caused the blank field left in\n> place of \"(encoding_arg)\" when it is empty to not overwrite\n> \"buf\" which then caused the value in \"buf\" to be copied into\n> dst_argv twice. The resulting git command failed if there was no\n> available revision named \"log\" as shown in the trace.\n>\n> From the TIG_TRACE log:\n> git log log --no-color --pretty=raw --parents --parents --\n> fatal: bad revision 'log'\n>\n> This fix works by teaching tig that when it is supplied with a\n> blank field in the source argument buffer that it should skip\n> over that field and continue instead of copying the previous\n> field value into the destination buffer a second time.\n>\n> github issue # 167\n>\n> Signed-off-by: Drew Northup\n> ---\n>\n> This should apply cleanly to the tig public master whether the\n> mkstemps() patch I wrote has been applied or not.\n>\n> tig.c | 9 +++++++--\n> 1 file changed, 7 insertions(+), 2 deletions(-)\n>\n> diff --git a/tig.c b/tig.c\n> index ba9ba98..1016cfe 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -3105,10 +3105,11 @@ static bool\n> format_append_arg(struct format_context *format, const char ***dst_argv, const char *arg)\n> {\n> \tformat->bufpos = 0;\n> +\tint len = 0;\n>\n> \twhile (arg) {\n> \t\tchar *next = strstr(arg, \"%(\");\n> -\t\tint len = next ? next - arg : strlen(arg);\n> +\t\tlen = next ? next - arg : strlen(arg);\n>\n> \t\tif (len&& !string_format_from(format->buf,&format->bufpos, \"%.*s\", len, arg))\n> \t\t\treturn FALSE;\n> @@ -3119,7 +3120,11 @@ format_append_arg(struct format_context *format, const char ***dst_argv, const c\n> \t\targ = next ? strchr(next, ')') + 1 : NULL;\n> \t}\n>\n> -\treturn argv_append(dst_argv, format->buf);\n> +\tif(len){\n> +\t\treturn argv_append(dst_argv, format->buf);\n> +\t} else {\n> +\t\treturn TRUE;\n> +\t}\n> }\n>\n> static bool\n\n\n-- \n--\n-Drew Northup\n--------------------------------------------------------------\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"223735","messageId":"CAFuPQ1K=hWUSCEhaGsuW8PeoGQcj6AoPo61zUTDbjLE802h+7g@mail.gmail.com","threadId":"34461","inReplyTo":"51E7EDE3.4080205@gmail.com","subject":"Re: [PATCH] TIG: Fix to reinstate proper operation with no arguments","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2013-07-19T04:07:05Z","receivedAt":"2013-07-19T04:07:05Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Thu, Jul 18, 2013 at 9:30 AM, Drew Northup wrote:\n>\n> Somehow this patch breaks the main view to not open the correct commit in diff view when is pressed. Back to the debugger...\n\nDoes this (possibly white-space damaged) patch work for you?\n\ndiff --git a/tig.c b/tig.c\nindex ba9ba98..74a2928 100644\n--- a/tig.c\n+++ b/tig.c\n@@ -3104,7 +3104,7 @@ format_expand_arg(struct format_context *format,\nconst char *name)\n static bool\n format_append_arg(struct format_context *format, const char\n***dst_argv, const char *arg)\n {\n- format->bufpos = 0;\n+ format->buf[0] = format->bufpos = 0;\n\n while (arg) {\n char *next = strstr(arg, \"%(\");\n\n\n--\nJonas Fonseca\n"},{"id":"223749","messageId":"CAM9Z-n=_vV2+TOXqj_p+wbuFVX8DEU8p_NtytqGepziLdJeong@mail.gmail.com","threadId":"34461","inReplyTo":"CAFuPQ1K=hWUSCEhaGsuW8PeoGQcj6AoPo61zUTDbjLE802h+7g@mail.gmail.com","subject":"Re: [PATCH] TIG: Fix to reinstate proper operation with no arguments","fromName":"Drew Northup","fromEmail":"n1xim.email@gmail.com","sentAt":"2013-07-19T11:55:04Z","receivedAt":"2013-07-19T11:55:04Z","isPatch":true,"sender":{"key":"n1xim.email@gmail.com","avatar":null},"body":"On Fri, Jul 19, 2013 at 12:07 AM, Jonas Fonseca wrote:\n> On Thu, Jul 18, 2013 at 9:30 AM, Drew Northup wrote:\n>>\n>> Somehow this patch breaks the main view to not open the correct commit in diff view when is pressed. Back to the debugger...\n>\n> Does this (possibly white-space damaged) patch work for you?\n\nI did look back at that sort of fix, but I kept getting compiler\nwarnings about a pointer conversion. Yes, it does work (I was actually\ninitializing the whole buffer to NULL), but I'd prefer doing so\nwithout the implicit int* conversion warnings.\n\n> diff --git a/tig.c b/tig.c\n> index ba9ba98..74a2928 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -3104,7 +3104,7 @@ format_expand_arg(struct format_context *format,\n> const char *name)\n> static bool\n> format_append_arg(struct format_context *format, const char\n> ***dst_argv, const char *arg)\n> {\n> - format->bufpos = 0;\n> + format->buf[0] = format->bufpos = 0;\n>\n> while (arg) {\n> char *next = strstr(arg, \"%(\");\n\n--\n-Drew Northup\n--------------------------------------------------------------\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"224035","messageId":"51EFC8B1.20405@gmail.com","threadId":"34461","inReplyTo":"CAFuPQ1K=hWUSCEhaGsuW8PeoGQcj6AoPo61zUTDbjLE802h+7g@mail.gmail.com","subject":"Re: [PATCH] TIG: Fix to reinstate proper operation with no arguments","fromName":"Drew Northup","fromEmail":"n1xim.email@gmail.com","sentAt":"2013-07-24T12:29:37Z","receivedAt":"2013-07-24T12:29:37Z","isPatch":true,"sender":{"key":"n1xim.email@gmail.com","avatar":null},"body":"On 07/19/2013 12:07 AM, Jonas Fonseca wrote:\n> On Thu, Jul 18, 2013 at 9:30 AM, Drew Northup wrote:\n>>\n>> Somehow this patch breaks the main view to not open the correct commit in diff view when is pressed. Back to the debugger...\n>\n> Does this (possibly white-space damaged) patch work for you?\n\nIt does, but I prefer to actually fix the problem this highlights and \nnot to paper it over. Patch V2 will be on the way shortly.\n\n> diff --git a/tig.c b/tig.c\n> index ba9ba98..74a2928 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -3104,7 +3104,7 @@ format_expand_arg(struct format_context *format,\n> const char *name)\n> static bool\n> format_append_arg(struct format_context *format, const char\n> ***dst_argv, const char *arg)\n> {\n> - format->bufpos = 0;\n> + format->buf[0] = format->bufpos = 0;\n>\n> while (arg) {\n> char *next = strstr(arg, \"%(\");\n>\n>\n> --\n> Jonas Fonseca\n\n\n-- \n--\n-Drew Northup\n--------------------------------------------------------------\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"224037","messageId":"1374670227-15677-1-git-send-email-n1xim.email@gmail.com","threadId":"34461","inReplyTo":"51EFC8B1.20405@gmail.com","subject":"[PATCH V2] TIG: Fix to reinstate proper operation with no arguments","fromName":"Drew Northup","fromEmail":"n1xim.email@gmail.com","sentAt":"2013-07-24T12:50:27Z","receivedAt":"2013-07-24T12:50:27Z","isPatch":true,"sender":{"key":"n1xim.email@gmail.com","avatar":null},"body":"Since c7d67ab running \"tig\" with no options has failed with the\nerror \"tig: No revisions match the given arguments.\" This was due\nto a change in how the arguments for the back-end git call was\nbeing constructed. This change caused the blank field left in\nplace of \"(encoding_arg)\" when it is empty to not overwrite\n\"buf\" which then caused the value in \"buf\" to be copied into\ndst_argv twice. The resulting git command failed if there was no\navailable revision named \"log\" as shown in the trace.\n\n>From the TIG_TRACE log:\ngit log log --no-color --pretty=raw --parents --parents --\nfatal: bad revision 'log'\n\nThis fix works by properly and fully initializing \"buf\" before each use.\n\ngithub issue # 167\n\nHelped-by: Jonas Fonseca \nSigned-off-by: Drew Northup \n---\n\nThis should apply cleanly to the tig public master whether the\nmkstemps() patch I wrote has been applied or not. (As of the time I\nam sending this for review I haven't yet applied the latest fix to\nmy mkstemps() patch that has been submitted for git itself.)\n\nThis time, knowing for sure now that format->buf is not being used in\nthe extant code path for any other purpose, I went ahead and\ninitialized the whole thing to be sure that we don't find any other\nghosts hiding in that buffer between uses. Just initializing the\nfirst byte fixes the near term problem but does not prevent the buffer\ninitialization issue that this bug highlighted from rising up again\nlater on.\n\n tig.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/tig.c b/tig.c\nindex ba9ba98..c65bc43 100644\n--- a/tig.c\n+++ b/tig.c\n@@ -3104,8 +3104,12 @@ format_expand_arg(struct format_context *format, const char *name)\n static bool\n format_append_arg(struct format_context *format, const char ***dst_argv, const char *arg)\n {\n+\tint i;\n \tformat->bufpos = 0;\n \n+\tfor (i = 0; i < SIZEOF_STR; i++)\n+\t\tformat->buf[i] = 0;\n+\n \twhile (arg) {\n \t\tchar *next = strstr(arg, \"%(\");\n \t\tint len = next ? next - arg : strlen(arg);\n-- \n1.8.0\n"},{"id":"224206","messageId":"CAFuPQ1L336HsJdtpL9OOA9+2MDNqZQa6nVu9-kYUhdnCVn13dw@mail.gmail.com","threadId":"34461","inReplyTo":"1374670227-15677-1-git-send-email-n1xim.email@gmail.com","subject":"Re: [PATCH V2] TIG: Fix to reinstate proper operation with no arguments","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2013-07-29T01:23:00Z","receivedAt":"2013-07-29T01:23:00Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Wed, Jul 24, 2013 at 8:50 AM, Drew Northup wrote:\n> This time, knowing for sure now that format->buf is not being used in\n> the extant code path for any other purpose, I went ahead and\n> initialized the whole thing to be sure that we don't find any other\n> ghosts hiding in that buffer between uses. Just initializing the\n> first byte fixes the near term problem but does not prevent the buffer\n> initialization issue that this bug highlighted from rising up again\n> later on.\n\nThanks applied with minor tidyup.\n\n> diff --git a/tig.c b/tig.c\n> index ba9ba98..c65bc43 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -3104,8 +3104,12 @@ format_expand_arg(struct format_context *format, const char *name)\n> static bool\n> format_append_arg(struct format_context *format, const char ***dst_argv, const char *arg)\n> {\n> + int i;\n\nAdded space after the declaration.\n\n> format->bufpos = 0;\n>\n> + for (i = 0; i < SIZEOF_STR; i++)\n\nChanged this to use sizeof(format->buf) instead.\n\n> + format->buf[i] = 0;\n> +\n> while (arg) {\n> char *next = strstr(arg, \"%(\");\n> int len = next ? next - arg : strlen(arg);\n> --\n> 1.8.0\n>\n\n-- \nJonas Fonseca\n"}]}