Re: [BUG] zsystem:34: flock: invalid timeout value: '0'

[Sebastian Gniazdowski <sgniazdowski@xxxxxxxxx>]

The patch looks good to me.

On Sat, 27 Jun 2020 at 09:13, Cedric Ware <cedric.ware__bml@xxxxxxxxxxxxxx>
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