zsh-workers
 help / color / mirror / code / Atom feed
* [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  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

* 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

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).