* [BUG] zsystem:34: flock: invalid timeout value: '0' @ 2020-06-25 17:49 Sebastian Gniazdowski 2020-06-26 14:16 ` Daniel Shahaf 0 siblings, 1 reply; 16+ messages in thread From: Sebastian Gniazdowski @ 2020-06-25 17:49 UTC (permalink / raw) To: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 424 bytes --] Hi I think that something happened to zsystem / flock, as the following returns the above error under Cygwin: $ zsystem flock -t 0 -f FD -e /home/SG/.zsh_nr1 zsystem:34: flock: invalid timeout value: '0' Timeout of value 0 is an acceptable one - it means no waiting for the lock, just try-to-lock and exit. -- Sebastian Gniazdowski IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zinit Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-25 17:49 [BUG] zsystem:34: flock: invalid timeout value: '0' Sebastian Gniazdowski @ 2020-06-26 14:16 ` Daniel Shahaf 2020-06-26 19:42 ` Sebastian Gniazdowski 0 siblings, 1 reply; 16+ messages in thread From: Daniel Shahaf @ 2020-06-26 14:16 UTC (permalink / raw) To: Sebastian Gniazdowski; +Cc: Zsh hackers list Sebastian Gniazdowski wrote on Thu, 25 Jun 2020 19:49 +0200: > Hi > I think that something happened to zsystem / flock, as the following > returns the above error under Cygwin: > > $ zsystem flock -t 0 -f FD -e /home/SG/.zsh_nr1 > zsystem:34: flock: invalid timeout value: '0' > > Timeout of value 0 is an acceptable one - it means no waiting for the lock, > just try-to-lock and exit. You've neglected to identify the environment it works in, the environment number it doesn't work in, and the reason you didn't do a bisect as courtesy prescribes. In this case, system.c gets so few commits that a bisect is almost a formality. The bug is likely to have been introduced by workers/45708. I recommend that you test the behaviour with and without that commit and reported the results. I further recommend that, if you have time, you also prepare a patch, or at least an expected-to-fail regression test (using the 'f' flag after the exit code). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-26 14:16 ` Daniel Shahaf @ 2020-06-26 19:42 ` Sebastian Gniazdowski 2020-06-27 1:47 ` Daniel Shahaf 0 siblings, 1 reply; 16+ messages in thread From: Sebastian Gniazdowski @ 2020-06-26 19:42 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 1629 bytes --] On Fri, 26 Jun 2020 at 16:16, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Sebastian Gniazdowski wrote on Thu, 25 Jun 2020 19:49 +0200: > > Hi > > I think that something happened to zsystem / flock, as the following > > returns the above error under Cygwin: > > > > $ zsystem flock -t 0 -f FD -e /home/SG/.zsh_nr1 > > zsystem:34: flock: invalid timeout value: '0' > > > > Timeout of value 0 is an acceptable one - it means no waiting for the > lock, > > just try-to-lock and exit. > > You've neglected to identify the environment it works in What do you mean? I don't have access to environment other than Cygwin and I've written about the Cygwin-environment to narrow this report. > the environment number it doesn't work in I've written what has to converge to the current HEAD. > and the reason you didn't do > a bisect as courtesy prescribes. > > In this case, system.c gets so few commits that a bisect is almost > a formality. The bug is likely to have been introduced by workers/45708. > I saw the subsecond timeout changes conversation/patch and assumed that it's the cause, thus I've left the interested party to be alerted by the title. > I recommend that you test the behaviour with and without that commit > and reported the results. > > I further recommend that, if you have time, you also prepare a patch, > or at least an expected-to-fail regression test (using the 'f' flag > after the exit code). > I might prepare a patch, but I also think that the involved party might do this. -- Sebastian Gniazdowski IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zinit Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-26 19:42 ` Sebastian Gniazdowski @ 2020-06-27 1:47 ` Daniel Shahaf 2020-06-27 2:04 ` Bart Schaefer 0 siblings, 1 reply; 16+ messages in thread From: Daniel Shahaf @ 2020-06-27 1:47 UTC (permalink / raw) To: Sebastian Gniazdowski; +Cc: Zsh hackers list Sebastian Gniazdowski wrote on Fri, 26 Jun 2020 21:42 +0200: > On Fri, 26 Jun 2020 at 16:16, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Sebastian Gniazdowski wrote on Thu, 25 Jun 2020 19:49 +0200: > > > Hi > > > I think that something happened to zsystem / flock, as the following > > > returns the above error under Cygwin: > > > > > > $ zsystem flock -t 0 -f FD -e /home/SG/.zsh_nr1 > > > zsystem:34: flock: invalid timeout value: '0' > > > > > > Timeout of value 0 is an acceptable one - it means no waiting for the > > > lock, > > > just try-to-lock and exit. > > > > You've neglected to identify the environment it works in > > > What do you mean? I don't have access to environment other than Cygwin Your readers don't know this. > and I've written about the Cygwin-environment to narrow this report. Yes, but you didn't state the relevant details (version numbers of zsh, Cygwin, and Windows) of either the working case or the non-working case, nor the OS name of the non-working case. > > and the reason you didn't do > > a bisect as courtesy prescribes. > > > > In this case, system.c gets so few commits that a bisect is almost > > a formality. The bug is likely to have been introduced by workers/45708. > > > > I saw the subsecond timeout changes conversation/patch and assumed that > it's the cause, thus I've left the interested party to be alerted by the > title. It's not clear to me what made you assume that patch was the cause. At any rate, what you should have done is triage your own bug report and state your findings. What you had in fact done is posted a bug report and left it for others to triage. The part in the middle where you say "I have triaged this and I think this is caused by 45708" makes all the difference. If you don't say it, it won't have happened as far as your readers are concerned. > > I recommend that you test the behaviour with and without that commit > > and reported the results. > > > > I further recommend that, if you have time, you also prepare a patch, > > or at least an expected-to-fail regression test (using the 'f' flag > > after the exit code). > > > > I might prepare a patch, but I also think that the involved party might do > this. +1 I'm not going to comment any further about the process aspects. Regarding the actual bug report, the next steps are for the aforementioned commit to be confirmed as causing an unintended behaviour change and for a test and a patch to be written. Daniel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 1:47 ` Daniel Shahaf @ 2020-06-27 2:04 ` Bart Schaefer 2020-06-27 7:13 ` Cedric Ware 0 siblings, 1 reply; 16+ messages in thread From: Bart Schaefer @ 2020-06-27 2:04 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Sebastian Gniazdowski, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 1023 bytes --] On Fri, Jun 26, 2020 at 6:48 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Regarding the actual bug report, the next steps are for the > aforementioned commit to be confirmed as causing an unintended > behaviour change and for a test and a patch to be written. > It's pretty obvious that the patch caused the change: + if (timeout < 1e-6 || timeout > 1073741823.) { + zwarnnam(nam, "flock: invalid timeout value: '%s'", + optarg); + return 1; + } Similarly: + if (timeout_param.u.d < 1 + || timeout_param.u.d > 0.999 * LONG_MAX) { + zwarnnam(nam, "flock: invalid interval value: '%s'", + optarg); + return 1; + } I don't know enough about dealing with the float-valued time specs to be sure what to do about it, i.e., why a limit above zero was considered necessary or whether zero needs to be a special case. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 2:04 ` Bart Schaefer @ 2020-06-27 7:13 ` Cedric Ware 2020-06-27 7:27 ` Roman Perepelitsa 2020-06-27 17:25 ` Sebastian Gniazdowski 0 siblings, 2 replies; 16+ messages in thread From: Cedric Ware @ 2020-06-27 7:13 UTC (permalink / raw) To: Bart Schaefer; +Cc: Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list Hello, Bart Schaefer (Friday 2020-06-26): > It's pretty obvious that the patch caused the change: > > + if (timeout < 1e-6 || timeout > 1073741823.) { > + zwarnnam(nam, "flock: invalid timeout value: '%s'", > + optarg); Yes, sorry, didn't catch the 0 case. > Similarly: > > + if (timeout_param.u.d < 1 > + || timeout_param.u.d > 0.999 * LONG_MAX) { > + zwarnnam(nam, "flock: invalid interval value: '%s'", > + optarg); I don't think so. This one is for the -i parameter, which is new, it's not a behavior change. And IMO it doesn't make sense to allow a zero interval. > I don't know enough about dealing with the float-valued time specs to be > sure what to do about it, i.e., why a limit above zero was considered > necessary or whether zero needs to be a special case. I didn't want to allow specifying a timeout below 1 microsecond, because that's the granularity of zsleep(). One could still allow it but silently treat it as 0. In any case, the 0 value itself should indeed have been allowed. Here's a quick patch below, I've tested it, but I don't have time right now to do much more. Best regards, Cedric Ware. diff --git a/Src/Modules/system.c b/Src/Modules/system.c index 972aa0767..638fe029e 100644 --- a/Src/Modules/system.c +++ b/Src/Modules/system.c @@ -591,13 +591,16 @@ bin_zsystem_flock(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) timeout_param.u.d : (double)timeout_param.u.l; /* + * timeout can be 0 (no wait) but not so small as to be + * less than a microsecond. * timeout must not overflow time_t, but little is known * about this type's limits. Conservatively limit to 2^30-1 * (34 years). Then it can only overflow if time_t is only * a 32-bit int and CLOCK_MONOTONIC is not supported, in which * case there is a Y2038 problem anyway. */ - if (timeout < 1e-6 || timeout > 1073741823.) { + if (timeout != 0. && + (timeout < 1e-6 || timeout > 1073741823.)) { zwarnnam(nam, "flock: invalid timeout value: '%s'", optarg); return 1; diff --git a/Test/V14system.ztst b/Test/V14system.ztst index b8af96cda..06512fe05 100644 --- a/Test/V14system.ztst +++ b/Test/V14system.ztst @@ -13,7 +13,9 @@ %test ( - zsystem flock -t 0.1 -i 0.000001 $tst_dir/file + zsystem flock -t 0 -i 0.000001 $tst_dir/file && + zsystem flock -t 0.1 -i 0.000001 $tst_dir/file && + zsystem flock -t 1 -i 0.000001 $tst_dir/file ) 0:zsystem flock valid time arguments ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 7:13 ` Cedric Ware @ 2020-06-27 7:27 ` Roman Perepelitsa 2020-06-27 17:01 ` Bart Schaefer 2020-06-27 20:38 ` Cedric Ware 2020-06-27 17:25 ` Sebastian Gniazdowski 1 sibling, 2 replies; 16+ messages in thread From: Roman Perepelitsa @ 2020-06-27 7:27 UTC (permalink / raw) To: Cedric Ware Cc: Bart Schaefer, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list On Sat, Jun 27, 2020 at 9:14 AM Cedric Ware <cedric.ware__bml@normalesup.org> wrote: > > I didn't want to allow specifying a timeout below 1 microsecond, > because that's the granularity of zsleep(). One could still allow it > but silently treat it as 0. Timeout strictly between 0 and 1 microsecond is not much different from timeout between 1 and 2 microseconds. You can either round up or down in both cases. Rounding up is the right thing to do for sleeps and timeouts because it allows you to provide a guarantee. For sleep (pseudo code): time start = now(); if (sleep(duration) != INTERRUPTED) assert(now - start() >= duration); For anything with a timeout (pseudo code): time start = now(); if (do_with_timeout(duration) == TIMEDOUT) assert(now - start() >= duration); If you round down, there is no non-trivial assertion that is guaranteed to hold and that doesn't expose the internal granularity of the duration. Roman. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 7:27 ` Roman Perepelitsa @ 2020-06-27 17:01 ` Bart Schaefer 2020-06-27 17:12 ` Roman Perepelitsa 2020-06-27 20:38 ` Cedric Ware 1 sibling, 1 reply; 16+ messages in thread From: Bart Schaefer @ 2020-06-27 17:01 UTC (permalink / raw) To: Roman Perepelitsa Cc: Cedric Ware, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 469 bytes --] On Sat, Jun 27, 2020 at 12:27 AM Roman Perepelitsa < roman.perepelitsa@gmail.com> wrote: > > Timeout strictly between 0 and 1 microsecond is not much different > from timeout between 1 and 2 microseconds. You can either round up or > down in both cases. Rounding up is the right thing to do for sleeps > and timeouts because it allows you to provide a guarantee. > I would be mildly surprised if this doesn't happen as part of the OS implementation of the operation. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 17:01 ` Bart Schaefer @ 2020-06-27 17:12 ` Roman Perepelitsa 2020-06-27 17:58 ` Bart Schaefer 0 siblings, 1 reply; 16+ messages in thread From: Roman Perepelitsa @ 2020-06-27 17:12 UTC (permalink / raw) To: Bart Schaefer Cc: Cedric Ware, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list On Sat, Jun 27, 2020 at 7:01 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Sat, Jun 27, 2020 at 12:27 AM Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: >> >> Timeout strictly between 0 and 1 microsecond is not much different >> from timeout between 1 and 2 microseconds. You can either round up or >> down in both cases. Rounding up is the right thing to do for sleeps >> and timeouts because it allows you to provide a guarantee. > > I would be mildly surprised if this doesn't happen as part of the OS implementation of the operation. I've a tendency to use an overly direct language that's often perceived by others (especially Americans) as aggressive. Perhaps I was overcautious this time. Let me try again. Any implementation of sleep or timeout must round up. flock in zsh doesn't round up but it must. Roman. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 17:12 ` Roman Perepelitsa @ 2020-06-27 17:58 ` Bart Schaefer 2020-06-27 18:09 ` Roman Perepelitsa 0 siblings, 1 reply; 16+ messages in thread From: Bart Schaefer @ 2020-06-27 17:58 UTC (permalink / raw) To: Roman Perepelitsa Cc: Cedric Ware, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 828 bytes --] On Sat, Jun 27, 2020 at 10:12 AM Roman Perepelitsa < roman.perepelitsa@gmail.com> wrote: > On Sat, Jun 27, 2020 at 7:01 PM Bart Schaefer <schaefer@brasslantern.com> > wrote: > > > > On Sat, Jun 27, 2020 at 12:27 AM Roman Perepelitsa < > roman.perepelitsa@gmail.com> wrote: > >> > >> Rounding up is the right thing to do for sleeps > >> and timeouts because it allows you to provide a guarantee. > > > > I would be mildly surprised if this doesn't happen as part of the OS > implementation of the operation. > > I've a tendency to use an overly direct language that's often > perceived by others (especially Americans) as aggressive. Perhaps I > was overcautious this time. No, that's not it. I just meant that the interval is ultimately implemented by calling nanosleep(), and I strongly suspect that nanosleep() rounds up. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 17:58 ` Bart Schaefer @ 2020-06-27 18:09 ` Roman Perepelitsa 0 siblings, 0 replies; 16+ messages in thread From: Roman Perepelitsa @ 2020-06-27 18:09 UTC (permalink / raw) To: Bart Schaefer Cc: Cedric Ware, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list >> On Sat, Jun 27, 2020 at 7:01 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > No, that's not it. I just meant that the interval is ultimately implemented by calling nanosleep(), and I strongly suspect that nanosleep() rounds up. I'm sure it does. My point is that flock in zsh must not reject timeouts between 0 and 1 microseconds. The right thing to do when a timeout falls between two steps of granularity is to round up. When converting any kind of floating point timeout/deadline to integers, it's also necessary to round up, never down. Roman. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 7:27 ` Roman Perepelitsa 2020-06-27 17:01 ` Bart Schaefer @ 2020-06-27 20:38 ` Cedric Ware 2020-06-28 9:27 ` Roman Perepelitsa 1 sibling, 1 reply; 16+ messages in thread From: Cedric Ware @ 2020-06-27 20:38 UTC (permalink / raw) To: Roman Perepelitsa Cc: Bart Schaefer, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list Roman Perepelitsa (Saturday 2020-06-27): > Timeout strictly between 0 and 1 microsecond is not much different > from timeout between 1 and 2 microseconds. You can either round up or > down in both cases. Rounding up is the right thing to do for sleeps > and timeouts because it allows you to provide a guarantee. That's a good point. I don't think we can offer an actual guarantee (or at least I don't have time to really think it through), but rounding up can be done, alternative patch below. Best, Cedric Ware. diff --git a/Src/Modules/system.c b/Src/Modules/system.c index 972aa0767..ecd4e2546 100644 --- a/Src/Modules/system.c +++ b/Src/Modules/system.c @@ -597,7 +597,7 @@ bin_zsystem_flock(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) * a 32-bit int and CLOCK_MONOTONIC is not supported, in which * case there is a Y2038 problem anyway. */ - if (timeout < 1e-6 || timeout > 1073741823.) { + if (timeout > 1073741823.) { zwarnnam(nam, "flock: invalid timeout value: '%s'", optarg); return 1; @@ -623,7 +623,7 @@ bin_zsystem_flock(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) timeout_param.type = MN_FLOAT; timeout_param.u.d = (double)timeout_param.u.l; } - timeout_param.u.d *= 1e6; + timeout_param.u.d = ceil(timeout_param.u.d * 1e6); if (timeout_param.u.d < 1 || timeout_param.u.d > 0.999 * LONG_MAX) { zwarnnam(nam, "flock: invalid interval value: '%s'", @@ -704,7 +704,7 @@ bin_zsystem_flock(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) zgettime_monotonic_if_available(&now); end.tv_sec = now.tv_sec; end.tv_nsec = now.tv_nsec; - end.tv_nsec += modf(timeout, &timeout_s) * 1000000000L; + end.tv_nsec += ceil(modf(timeout, &timeout_s) * 1000000000L); end.tv_sec += timeout_s; if (end.tv_nsec >= 1000000000L) { end.tv_nsec -= 1000000000L; diff --git a/Test/V14system.ztst b/Test/V14system.ztst index b8af96cda..100daab08 100644 --- a/Test/V14system.ztst +++ b/Test/V14system.ztst @@ -13,27 +13,26 @@ %test ( - zsystem flock -t 0.1 -i 0.000001 $tst_dir/file + zsystem flock -t 0 -i 0.000001 $tst_dir/file && + zsystem flock -t 0.1 -i 0.000001 $tst_dir/file && + zsystem flock -t 0.1 -i 0.0000001 $tst_dir/file && + zsystem flock -t 1 -i 0.000001 $tst_dir/file ) 0:zsystem flock valid time arguments ( - zsystem flock -t -1 $tst_dir/file || - zsystem flock -t 0.49e-6 $tst_dir/file || zsystem flock -t 1073741824 $tst_dir/file || zsystem flock -t 1e100 $tst_dir/file || zsystem flock -i -1 $tst_dir/file || - zsystem flock -i 0.49e-6 $tst_dir/file || + zsystem flock -i 0 $tst_dir/file || zsystem flock -i 1e100 $tst_dir/file ) 1:zsystem flock invalid time arguments -?(eval):zsystem:2: flock: invalid timeout value: '-1' -?(eval):zsystem:3: flock: invalid timeout value: '0.49e-6' -?(eval):zsystem:4: flock: invalid timeout value: '1073741824' -?(eval):zsystem:5: flock: invalid timeout value: '1e100' -?(eval):zsystem:6: flock: invalid interval value: '-1' -?(eval):zsystem:7: flock: invalid interval value: '0.49e-6' -?(eval):zsystem:8: flock: invalid interval value: '1e100' +?(eval):zsystem:2: flock: invalid timeout value: '1073741824' +?(eval):zsystem:3: flock: invalid timeout value: '1e100' +?(eval):zsystem:4: flock: invalid interval value: '-1' +?(eval):zsystem:5: flock: invalid interval value: '0' +?(eval):zsystem:6: flock: invalid interval value: '1e100' ( # Lock file for 1 second in the background. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 20:38 ` Cedric Ware @ 2020-06-28 9:27 ` Roman Perepelitsa 2020-07-10 15:28 ` Sebastian Gniazdowski 0 siblings, 1 reply; 16+ messages in thread From: Roman Perepelitsa @ 2020-06-28 9:27 UTC (permalink / raw) To: Cedric Ware Cc: Bart Schaefer, Daniel Shahaf, Sebastian Gniazdowski, Zsh hackers list On Sat, Jun 27, 2020 at 10:38 PM Cedric Ware <cedric.ware__bml@normalesup.org> wrote: > > rounding up can be done, alternative patch below. LGTM. Roman. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-28 9:27 ` Roman Perepelitsa @ 2020-07-10 15:28 ` Sebastian Gniazdowski 2020-07-11 5:18 ` dana 0 siblings, 1 reply; 16+ messages in thread From: Sebastian Gniazdowski @ 2020-07-10 15:28 UTC (permalink / raw) To: Roman Perepelitsa Cc: Cedric Ware, Bart Schaefer, Daniel Shahaf, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 462 bytes --] Bumping this up as I'm worried why the patch hasn't been applied yet, has it gone lost? On Sun, 28 Jun 2020 at 11:28, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: > On Sat, Jun 27, 2020 at 10:38 PM Cedric Ware > <cedric.ware__bml@normalesup.org> wrote: > > > > rounding up can be done, alternative patch below. > > LGTM. > > Roman. > -- Sebastian Gniazdowski IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zinit Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-07-10 15:28 ` Sebastian Gniazdowski @ 2020-07-11 5:18 ` dana 0 siblings, 0 replies; 16+ messages in thread From: dana @ 2020-07-11 5:18 UTC (permalink / raw) To: Sebastian Gniazdowski; +Cc: Cedric Ware, Zsh hackers list On 10 Jul 2020, at 10:28, Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > Bumping this up as I'm worried why the patch hasn't been applied yet, has > it gone lost? Applied dana ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [BUG] zsystem:34: flock: invalid timeout value: '0' 2020-06-27 7:13 ` Cedric Ware 2020-06-27 7:27 ` Roman Perepelitsa @ 2020-06-27 17:25 ` Sebastian Gniazdowski 1 sibling, 0 replies; 16+ messages in thread From: Sebastian Gniazdowski @ 2020-06-27 17:25 UTC (permalink / raw) To: Cedric Ware; +Cc: Bart Schaefer, Daniel Shahaf, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 3295 bytes --] The patch looks good to me. On Sat, 27 Jun 2020 at 09:13, Cedric Ware <cedric.ware__bml@normalesup.org> wrote: > > Hello, > > Bart Schaefer (Friday 2020-06-26): > > It's pretty obvious that the patch caused the change: > > > > + if (timeout < 1e-6 || timeout > 1073741823.) { > > + zwarnnam(nam, "flock: invalid timeout value: '%s'", > > + optarg); > > Yes, sorry, didn't catch the 0 case. > > > Similarly: > > > > + if (timeout_param.u.d < 1 > > + || timeout_param.u.d > 0.999 * LONG_MAX) { > > + zwarnnam(nam, "flock: invalid interval value: '%s'", > > + optarg); > > I don't think so. This one is for the -i parameter, which is new, > it's not a behavior change. And IMO it doesn't make sense to allow > a zero interval. > > > I don't know enough about dealing with the float-valued time specs to be > > sure what to do about it, i.e., why a limit above zero was considered > > necessary or whether zero needs to be a special case. > > I didn't want to allow specifying a timeout below 1 microsecond, > because that's the granularity of zsleep(). One could still allow it > but silently treat it as 0. In any case, the 0 value itself should > indeed have been allowed. Here's a quick patch below, I've tested it, > but I don't have time right now to do much more. > > Best regards, > Cedric Ware. > > > diff --git a/Src/Modules/system.c b/Src/Modules/system.c > index 972aa0767..638fe029e 100644 > --- a/Src/Modules/system.c > +++ b/Src/Modules/system.c > @@ -591,13 +591,16 @@ bin_zsystem_flock(char *nam, char **args, > UNUSED(Options ops), UNUSED(int func)) > timeout_param.u.d : (double)timeout_param.u.l; > > /* > + * timeout can be 0 (no wait) but not so small as to be > + * less than a microsecond. > * timeout must not overflow time_t, but little is known > * about this type's limits. Conservatively limit to > 2^30-1 > * (34 years). Then it can only overflow if time_t is only > * a 32-bit int and CLOCK_MONOTONIC is not supported, in > which > * case there is a Y2038 problem anyway. > */ > - if (timeout < 1e-6 || timeout > 1073741823.) { > + if (timeout != 0. && > + (timeout < 1e-6 || timeout > 1073741823.)) { > zwarnnam(nam, "flock: invalid timeout value: '%s'", > optarg); > return 1; > diff --git a/Test/V14system.ztst b/Test/V14system.ztst > index b8af96cda..06512fe05 100644 > --- a/Test/V14system.ztst > +++ b/Test/V14system.ztst > @@ -13,7 +13,9 @@ > %test > > ( > - zsystem flock -t 0.1 -i 0.000001 $tst_dir/file > + zsystem flock -t 0 -i 0.000001 $tst_dir/file && > + zsystem flock -t 0.1 -i 0.000001 $tst_dir/file && > + zsystem flock -t 1 -i 0.000001 $tst_dir/file > ) > 0:zsystem flock valid time arguments > > -- Sebastian Gniazdowski IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zinit Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-07-11 5:17 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-25 17:49 [BUG] zsystem:34: flock: invalid timeout value: '0' Sebastian Gniazdowski 2020-06-26 14:16 ` Daniel Shahaf 2020-06-26 19:42 ` Sebastian Gniazdowski 2020-06-27 1:47 ` Daniel Shahaf 2020-06-27 2:04 ` Bart Schaefer 2020-06-27 7:13 ` Cedric Ware 2020-06-27 7:27 ` Roman Perepelitsa 2020-06-27 17:01 ` Bart Schaefer 2020-06-27 17:12 ` Roman Perepelitsa 2020-06-27 17:58 ` Bart Schaefer 2020-06-27 18:09 ` Roman Perepelitsa 2020-06-27 20:38 ` Cedric Ware 2020-06-28 9:27 ` Roman Perepelitsa 2020-07-10 15:28 ` Sebastian Gniazdowski 2020-07-11 5:18 ` dana 2020-06-27 17:25 ` Sebastian Gniazdowski
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).