{"thread":{"id":"7744","subject":"[RFD PATCH] git-fetch--tool and \"insanely\" long actions","startedAt":"2007-04-20T01:05:58Z","lastAt":"2007-04-20T07:40:39Z","messageCount":4,"participants":["A Large Angry SCM","Julian Phillips"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"39928","messageId":"462811F6.9060503@gmail.com","threadId":"7744","inReplyTo":null,"subject":"[RFD PATCH] git-fetch--tool and \"insanely\" long actions","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2007-04-20T01:05:58Z","receivedAt":"2007-04-20T01:05:58Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"This fixes a problem my repository mirroring script has been having since\nthe git-fetch--tool was added to master in the middle of March. However,\nit is not a proper fix since it causes actual errors from snprintf() to be\nignored. A proper fix is complicated by the lack of a consistent indicator\nthat the buffer is too small across snprintf() implementations.\n\n\n builtin-fetch--tool.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex e9d6764..173dd4f 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -44,7 +44,7 @@ static int update_ref(const char *action,\n \t\trla = \"(reflog update)\";\n \tlen = snprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n \tif (sizeof(msg) <= len)\n-\t\tdie(\"insanely long action\");\n+\t\tmsg[sizeof(msg)-1] = '\\0';\n \tlock = lock_any_ref_for_update(refname, oldval);\n \tif (!lock)\n \t\treturn 1;\n"},{"id":"39930","messageId":"20070420013411.26401.77137.julian@quantumfyre.co.uk","threadId":"7744","inReplyTo":"462811F6.9060503@gmail.com","subject":"Re: [RFD PATCH] git-fetch--tool and \"insanely\" long actions","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-20T01:34:04Z","receivedAt":"2007-04-20T01:34:04Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Thu, 19 Apr 2007, A Large Angry SCM wrote:\n\n> This fixes a problem my repository mirroring script has been having since\n> the git-fetch--tool was added to master in the middle of March. However,\n> it is not a proper fix since it causes actual errors from snprintf() to be\n> ignored. A proper fix is complicated by the lack of a consistent indicator\n> that the buffer is too small across snprintf() implementations.\n.\n.\n.\n>       if (sizeof(msg) <= len)\n> -             die(\"insanely long action\");\n> +             msg[sizeof(msg)-1] = '\\0';\n\nOr you could just let the whole thing through?\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex e9d6764..9b5ae9f 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -36,21 +36,26 @@ static int update_ref(const char *action,\n \t\t      unsigned char *oldval)\n {\n \tint len;\n-\tchar msg[1024];\n+\tchar buffer[1024];\n+\tint ret = 0;\n+\tchar *msg = buffer;\n \tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n \tstatic struct ref_lock *lock;\n \n \tif (!rla)\n \t\trla = \"(reflog update)\";\n-\tlen = snprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n-\tif (sizeof(msg) <= len)\n-\t\tdie(\"insanely long action\");\n+\tlen = strlen(rla) + strlen(action) + 3;\n+\tif (len > sizeof(buffer))\n+\t\tmsg = xmalloc(len);\n+\tsnprintf(msg, len, \"%s: %s\", rla, action);\n \tlock = lock_any_ref_for_update(refname, oldval);\n \tif (!lock)\n-\t\treturn 1;\n+\t\tret = 1;\n \tif (write_ref_sha1(lock, sha1, msg) < 0)\n-\t\treturn 1;\n-\treturn 0;\n+\t\tret = 1;\n+\tif (msg != buffer)\n+\t\tfree(msg);\n+\treturn ret;\n }\n \n static int update_local_ref(const char *name,\n-- \n1.5.1.1\n"},{"id":"39932","messageId":"46281D0B.7080802@gmail.com","threadId":"7744","inReplyTo":"20070420013411.26401.77137.julian@quantumfyre.co.uk","subject":"Re: [RFD PATCH] git-fetch--tool and \"insanely\" long actions","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2007-04-20T01:53:15Z","receivedAt":"2007-04-20T01:53:15Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Julian Phillips wrote:\n> On Thu, 19 Apr 2007, A Large Angry SCM wrote:\n> \n>> This fixes a problem my repository mirroring script has been having since\n>> the git-fetch--tool was added to master in the middle of March. However,\n>> it is not a proper fix since it causes actual errors from snprintf() to be\n>> ignored. A proper fix is complicated by the lack of a consistent indicator\n>> that the buffer is too small across snprintf() implementations.\n> .\n> .\n> .\n>>       if (sizeof(msg) <= len)\n>> -             die(\"insanely long action\");\n>> +             msg[sizeof(msg)-1] = '\\0';\n> \n> Or you could just let the whole thing through?\n> \n> diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\n> index e9d6764..9b5ae9f 100644\n> --- a/builtin-fetch--tool.c\n> +++ b/builtin-fetch--tool.c\n> @@ -36,21 +36,26 @@ static int update_ref(const char *action,\n>  \t\t      unsigned char *oldval)\n>  {\n>  \tint len;\n> -\tchar msg[1024];\n> +\tchar buffer[1024];\n> +\tint ret = 0;\n> +\tchar *msg = buffer;\n>  \tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n>  \tstatic struct ref_lock *lock;\n>  \n>  \tif (!rla)\n>  \t\trla = \"(reflog update)\";\n> -\tlen = snprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n> -\tif (sizeof(msg) <= len)\n> -\t\tdie(\"insanely long action\");\n> +\tlen = strlen(rla) + strlen(action) + 3;\n> +\tif (len > sizeof(buffer))\n> +\t\tmsg = xmalloc(len);\n> +\tsnprintf(msg, len, \"%s: %s\", rla, action);\n>  \tlock = lock_any_ref_for_update(refname, oldval);\n>  \tif (!lock)\n> -\t\treturn 1;\n> +\t\tret = 1;\n>  \tif (write_ref_sha1(lock, sha1, msg) < 0)\n> -\t\treturn 1;\n> -\treturn 0;\n> +\t\tret = 1;\n> +\tif (msg != buffer)\n> +\t\tfree(msg);\n> +\treturn ret;\n>  }\n>  \n>  static int update_local_ref(const char *name,\n\n\nSee the last sentence in my original message. Yours also ignores errors \nfrom snprintf().\n"},{"id":"39948","messageId":"Pine.LNX.4.64.0704200837460.29434@beast.quantumfyre.co.uk","threadId":"7744","inReplyTo":"46281D0B.7080802@gmail.com","subject":"Re: [RFD PATCH] git-fetch--tool and \"insanely\" long actions","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-20T07:40:39Z","receivedAt":"2007-04-20T07:40:39Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Thu, 19 Apr 2007, A Large Angry SCM wrote:\n\n> Julian Phillips wrote:\n>>  On Thu, 19 Apr 2007, A Large Angry SCM wrote:\n>> \n>> >  This fixes a problem my repository mirroring script has been having \n>> >  since\n>> >  the git-fetch--tool was added to master in the middle of March. However,\n>> >  it is not a proper fix since it causes actual errors from snprintf() to \n>> >  be\n>> >  ignored. A proper fix is complicated by the lack of a consistent \n>> >  indicator\n>> >  that the buffer is too small across snprintf() implementations.\n\n>\n> See the last sentence in my original message. Yours also ignores errors from \n> snprintf().\n>\n\nWell your last sentence was about the buffer being too small.  The change \nI made means it won't ever be too small.  True that it doesn't check for \nother errors.\n\n-- \nJulian\n\n  ---\nYou will be successful in your work.\n"}]}