# [RFD PATCH] git-fetch--tool and "insanely" long actions 4 messages from 2007-04-20 to 2007-04-20. Participants: A Large Angry SCM, Julian Phillips. Thread: https://gitlist.dev/t/7744 ## A Large Angry SCM, 2007-04-20 01:05 Subject: [RFD PATCH] git-fetch--tool and "insanely" long actions Message-ID: <462811F6.9060503@gmail.com> URL: https://gitlist.dev/e/462811F6.9060503%40gmail.com ``` This fixes a problem my repository mirroring script has been having since the git-fetch--tool was added to master in the middle of March. However, it is not a proper fix since it causes actual errors from snprintf() to be ignored. A proper fix is complicated by the lack of a consistent indicator that the buffer is too small across snprintf() implementations. builtin-fetch--tool.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c index e9d6764..173dd4f 100644 --- a/builtin-fetch--tool.c +++ b/builtin-fetch--tool.c @@ -44,7 +44,7 @@ static int update_ref(const char *action, rla = "(reflog update)"; len = snprintf(msg, sizeof(msg), "%s: %s", rla, action); if (sizeof(msg) <= len) - die("insanely long action"); + msg[sizeof(msg)-1] = '\0'; lock = lock_any_ref_for_update(refname, oldval); if (!lock) return 1; ``` ## Julian Phillips, 2007-04-20 01:34 Subject: Re: [RFD PATCH] git-fetch--tool and "insanely" long actions Message-ID: <20070420013411.26401.77137.julian@quantumfyre.co.uk> URL: https://gitlist.dev/e/20070420013411.26401.77137.julian%40quantumfyre.co.uk In-Reply-To: <462811F6.9060503@gmail.com> ``` On Thu, 19 Apr 2007, A Large Angry SCM wrote: > This fixes a problem my repository mirroring script has been having since > the git-fetch--tool was added to master in the middle of March. However, > it is not a proper fix since it causes actual errors from snprintf() to be > ignored. A proper fix is complicated by the lack of a consistent indicator > that the buffer is too small across snprintf() implementations. . . . > if (sizeof(msg) <= len) > - die("insanely long action"); > + msg[sizeof(msg)-1] = '\0'; Or you could just let the whole thing through? diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c index e9d6764..9b5ae9f 100644 --- a/builtin-fetch--tool.c +++ b/builtin-fetch--tool.c @@ -36,21 +36,26 @@ static int update_ref(const char *action, unsigned char *oldval) { int len; - char msg[1024]; + char buffer[1024]; + int ret = 0; + char *msg = buffer; char *rla = getenv("GIT_REFLOG_ACTION"); static struct ref_lock *lock; if (!rla) rla = "(reflog update)"; - len = snprintf(msg, sizeof(msg), "%s: %s", rla, action); - if (sizeof(msg) <= len) - die("insanely long action"); + len = strlen(rla) + strlen(action) + 3; + if (len > sizeof(buffer)) + msg = xmalloc(len); + snprintf(msg, len, "%s: %s", rla, action); lock = lock_any_ref_for_update(refname, oldval); if (!lock) - return 1; + ret = 1; if (write_ref_sha1(lock, sha1, msg) < 0) - return 1; - return 0; + ret = 1; + if (msg != buffer) + free(msg); + return ret; } static int update_local_ref(const char *name, -- 1.5.1.1 ``` ## A Large Angry SCM, 2007-04-20 01:53 Subject: Re: [RFD PATCH] git-fetch--tool and "insanely" long actions Message-ID: <46281D0B.7080802@gmail.com> URL: https://gitlist.dev/e/46281D0B.7080802%40gmail.com In-Reply-To: <20070420013411.26401.77137.julian@quantumfyre.co.uk> ``` Julian Phillips wrote: > On Thu, 19 Apr 2007, A Large Angry SCM wrote: > >> This fixes a problem my repository mirroring script has been having since >> the git-fetch--tool was added to master in the middle of March. However, >> it is not a proper fix since it causes actual errors from snprintf() to be >> ignored. A proper fix is complicated by the lack of a consistent indicator >> that the buffer is too small across snprintf() implementations. > . > . > . >> if (sizeof(msg) <= len) >> - die("insanely long action"); >> + msg[sizeof(msg)-1] = '\0'; > > Or you could just let the whole thing through? > > diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c > index e9d6764..9b5ae9f 100644 > --- a/builtin-fetch--tool.c > +++ b/builtin-fetch--tool.c > @@ -36,21 +36,26 @@ static int update_ref(const char *action, > unsigned char *oldval) > { > int len; > - char msg[1024]; > + char buffer[1024]; > + int ret = 0; > + char *msg = buffer; > char *rla = getenv("GIT_REFLOG_ACTION"); > static struct ref_lock *lock; > > if (!rla) > rla = "(reflog update)"; > - len = snprintf(msg, sizeof(msg), "%s: %s", rla, action); > - if (sizeof(msg) <= len) > - die("insanely long action"); > + len = strlen(rla) + strlen(action) + 3; > + if (len > sizeof(buffer)) > + msg = xmalloc(len); > + snprintf(msg, len, "%s: %s", rla, action); > lock = lock_any_ref_for_update(refname, oldval); > if (!lock) > - return 1; > + ret = 1; > if (write_ref_sha1(lock, sha1, msg) < 0) > - return 1; > - return 0; > + ret = 1; > + if (msg != buffer) > + free(msg); > + return ret; > } > > static int update_local_ref(const char *name, See the last sentence in my original message. Yours also ignores errors from snprintf(). ``` ## Julian Phillips, 2007-04-20 07:40 Subject: Re: [RFD PATCH] git-fetch--tool and "insanely" long actions Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0704200837460.29434%40beast.quantumfyre.co.uk In-Reply-To: <46281D0B.7080802@gmail.com> ``` On Thu, 19 Apr 2007, A Large Angry SCM wrote: > Julian Phillips wrote: >> On Thu, 19 Apr 2007, A Large Angry SCM wrote: >> >> > This fixes a problem my repository mirroring script has been having >> > since >> > the git-fetch--tool was added to master in the middle of March. However, >> > it is not a proper fix since it causes actual errors from snprintf() to >> > be >> > ignored. A proper fix is complicated by the lack of a consistent >> > indicator >> > that the buffer is too small across snprintf() implementations. > > See the last sentence in my original message. Yours also ignores errors from > snprintf(). > Well your last sentence was about the buffer being too small. The change I made means it won't ever be too small. True that it doesn't check for other errors. -- Julian --- You will be successful in your work. ```