[PATCH] upload-pack: keep poll(2)'s timeout to -1
Keep poll's timeout at -1 when uploadpack.keepalive = 0, instead of
setting it to -1000, since some pedantic old systems (eg HP-UX) and
the gnulib compat/poll will treat only -1 as the valid value for
an infinite timeout.
Signed-off-by: Edward Thomson ethom...@microsoft.com
---
upload-pack.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/upload-pack.c b/upload-pack.c
index 01de944..433211a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -167,7 +167,9 @@ static void create_pack_file(void)
if (!pollsize)
break;
- ret = poll(pfd, pollsize, 1000 * keepalive);
+ ret = poll(pfd, pollsize,
+ keepalive 0 ? -1 : 1000 * keepalive);
+
if (ret 0) {
if (errno != EINTR) {
error(poll failed, resuming: %s,
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
On Fri, Aug 22, 2014 at 03:19:11PM +, Edward Thomson wrote: Keep poll's timeout at -1 when uploadpack.keepalive = 0, instead of setting it to -1000, since some pedantic old systems (eg HP-UX) and the gnulib compat/poll will treat only -1 as the valid value for an infinite timeout. That makes sense, and POSIX only specifies the behavior for -1 anyway. The patch itself looks obviously correct. Thanks. Since we're now translating the keepalive value, and since there's no way to set it to 0 (nor would that really have any meaning), I guess we could switch the internal no keepalive value to 0, and do: ret = poll(pfd, pollsize, keepalive ? 1000 * keepalive : -1); which would let us avoid setting it to -1 in some other spots. I dunno if that actually makes a real difference to maintainability, though. Either way: Acked-by: Jeff King p...@peff.net -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
Jeff King p...@peff.net writes:
Since we're now translating the keepalive value, and since there's no
way to set it to 0 (nor would that really have any meaning), I guess
we could switch the internal no keepalive value to 0, and do:
ret = poll(pfd, pollsize, keepalive ? 1000 * keepalive : -1);
which would let us avoid setting it to -1 in some other spots. I dunno
if that actually makes a real difference to maintainability, though.
Where we parse and set the value of the variable, we do this:
else if (!strcmp(uploadpack.keepalive, var)) {
keepalive = git_config_int(var, value);
if (!keepalive)
keepalive = -1;
}
The condition may have to become if (keepalive = 0).
Either way:
Acked-by: Jeff King p...@peff.net
-Peff
Yeah, either way, the patch as-posted is good. Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
On Fri, Aug 22, 2014 at 08:56:12AM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Since we're now translating the keepalive value, and since there's no
way to set it to 0 (nor would that really have any meaning), I guess
we could switch the internal no keepalive value to 0, and do:
ret = poll(pfd, pollsize, keepalive ? 1000 * keepalive : -1);
which would let us avoid setting it to -1 in some other spots. I dunno
if that actually makes a real difference to maintainability, though.
Where we parse and set the value of the variable, we do this:
else if (!strcmp(uploadpack.keepalive, var)) {
keepalive = git_config_int(var, value);
if (!keepalive)
keepalive = -1;
}
The condition may have to become if (keepalive = 0).
Yeah, I wasn't thinking we would get negative values from the user (we
don't document them at all), but we should probably do something
sensible. Let's just leave it at Ed's patch.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
On Fri, Aug 22, 2014 at 12:03:34PM -0400, Jeff King wrote: Yeah, I wasn't thinking we would get negative values from the user (we don't document them at all), but we should probably do something sensible. Let's just leave it at Ed's patch. Thanks, both. Apologies for the dumb question: is there anything additional that I need to do (repost with your Acked-by, for example) or is this adequate as-is? Thanks- -ed -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
Jeff King p...@peff.net writes: On Fri, Aug 22, 2014 at 03:19:11PM +, Edward Thomson wrote: Keep poll's timeout at -1 when uploadpack.keepalive = 0, instead of setting it to -1000, since some pedantic old systems (eg HP-UX) and the gnulib compat/poll will treat only -1 as the valid value for an infinite timeout. That makes sense, and POSIX only specifies the behavior for -1 anyway. The patch itself looks obviously correct. Thanks. Since we're now translating the keepalive value, and since there's no way to set it to 0 (nor would that really have any meaning), I guess we could switch the internal no keepalive value to 0, and do: ret = poll(pfd, pollsize, keepalive ? 1000 * keepalive : -1); which would let us avoid setting it to -1 in some other spots. I dunno if that actually makes a real difference to maintainability, though. Either way: Acked-by: Jeff King p...@peff.net -Peff There is 1000 * wakeup in credential-cache--daemon.c, by the way. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
Junio C Hamano gits...@pobox.com writes: There is 1000 * wakeup in credential-cache--daemon.c, by the way. Ah, nevermind. That uses an expiration computed, not some we can choose to block indefinitely configuration. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] upload-pack: keep poll(2)'s timeout to -1
Edward Thomson ethom...@edwardthomson.com writes: On Fri, Aug 22, 2014 at 12:03:34PM -0400, Jeff King wrote: Yeah, I wasn't thinking we would get negative values from the user (we don't document them at all), but we should probably do something sensible. Let's just leave it at Ed's patch. Thanks, both. Apologies for the dumb question: is there anything additional that I need to do (repost with your Acked-by, for example) or is this adequate as-is? I've picked it up and queued it on 'pu'. Thanks. commit 6c71f8b0d3d39beffe050f92f33a25dc30dffca3 Author: Edward Thomson ethom...@edwardthomson.com Date: Fri Aug 22 15:19:11 2014 + upload-pack: keep poll(2)'s timeout to -1 Keep poll's timeout at -1 when uploadpack.keepalive = 0, instead of setting it to -1000, since some pedantic old systems (eg HP-UX) and the gnulib compat/poll will treat only -1 as the valid value for an infinite timeout. Signed-off-by: Edward Thomson ethom...@microsoft.com Acked-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
