From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE,RDNS_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 Received: (qmail 2326 invoked from network); 15 Mar 2020 16:55:06 -0000 Received-SPF: pass (primenet.com.au: domain of zsh.org designates 203.24.36.2 as permitted sender) receiver=inbox.vuxu.org; client-ip=203.24.36.2 envelope-from= Received: from unknown (HELO primenet.com.au) (203.24.36.2) by inbox.vuxu.org with ESMTP; 15 Mar 2020 16:55:06 -0000 Received: (qmail 22029 invoked by alias); 15 Mar 2020 16:54:57 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 45562 Received: (qmail 27647 invoked by uid 1010); 15 Mar 2020 16:54:57 -0000 X-Qmail-Scanner-Diagnostics: from out2-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.2/25751. spamassassin: 3.4.2. Clear:RC:0(66.111.4.26):SA:0(-2.6/5.0):. Processed in 4.411167 secs); 15 Mar 2020 16:54:57 -0000 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrudeftddgvddutdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjfgesth dttddttdervdenucfhrhhomhepffgrnhhivghlucfuhhgrhhgrfhcuoegurdhssegurghn ihgvlhdrshhhrghhrghfrdhnrghmvgeqnecukfhppedutdelrdeigedrvddvrdduieekne cuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepugdrshes uggrnhhivghlrdhshhgrhhgrfhdrnhgrmhgv X-ME-Proxy: Date: Sun, 15 Mar 2020 16:54:10 +0000 From: Daniel Shahaf To: Cedric Ware Cc: dana , zsh-workers@zsh.org Subject: Re: [PATCH] Enable sub-second timeout in zsystem flock Message-ID: <20200315165410.GA30241@tarpaulin.shahaf.local2> References: <20200106173030.eb2pg4rhhgysh35r@phare.normalesup.org> <20200111154143.fjtwgfnztqfmkyda@phare.normalesup.org> <20200308183907.mxnhqrr2uflwooax@phare.normalesup.org> <20200314210454.hp562smyqv3ew255@phare.normalesup.org> <20200315005036.45bc846b@tarpaulin.shahaf.local2> <20200315160324.dstgtmajzwxpaccn@phare.normalesup.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200315160324.dstgtmajzwxpaccn@phare.normalesup.org> User-Agent: Mutt/1.10.1 (2018-07-13) Cedric Ware wrote on Sun, Mar 15, 2020 at 17:03:24 +0100: > Daniel Shahaf (Sunday 2020-03-15): > > Cedric Ware wrote on Sat, 14 Mar 2020 22:04 +0100: > > > +++ zsh-5.8/Doc/Zsh/mod_system.yo 2020-03-08 18:39:32.672683779 +0100 > > > @@ -166,7 +166,7 @@ > > > printed in the last case, but the parameter tt(ERRNO) will reflect > > > the error that occurred. > > > ) > > > -xitem(tt(zsystem flock) [ tt(-t) var(timeout) ] [ tt(-f) var(var) ] [tt(-er)] var(file)) > > > +xitem(tt(zsystem flock) [ tt(-t) var(timeout) ] [ tt(-i) var(interval) ] [ tt(-f) var(var) ] [tt(-er)] var(file)) > > > item(tt(zsystem flock -u) var(fd_expr))( > > > > Please s/var(var)/var(interval)/ in the body paragraphs of the item()(). > > I don't understand. Option -f's parameter is actually called "var". > The only occurrences of "var(var)" here are about -f, I didn't add them. My bad; I'd misread the diff. Sorry for the noise. > > > + timeout_tmp = timeout_param.u.d * 1e6; > > > + if ((timeout_tmp < 1) || (timeout_tmp > ZLONG_MAX / 2)) { > > > + zwarnnam(nam, "flock: invalid timeout value"); > > > > I think the invalid value should be included in the error message, > > either as a number, or as a string if needed: "flock: invalid timeout > > value: '%s'". > > Done, but it had to be as a string, because I didn't see a way to use > a zlong in zwarnnam(). That doesn't seem to be supported. It could be added, if needed. > > dana, I see you advocated the opposite approach in 45283. What's your > > rationale? > > I think that was about my earlier attempt to include the actual limit > in the error message, not the input. I reverted to a generic message, > because I don't know how to make the test script check for an error > message that depends on the system/compiler options. How would the message depend on the system/compiler options? In any case, you might be able to address this by specifying the expectations as patterns: # Each line of a '>' and '?' chunk may be preceded by a '*', so the line # starts '*>' or '*?'. This signifies that for any line with '*' in front # the actual output should be pattern matched against the corresponding # lines in the test output. Each line following '>' or '?' must be a # valid pattern, so characters special to patterns such as parentheses # must be quoted with a backslash. The EXTENDED_GLOB option is used for # all such patterns. > > > + zlong now; > > > + zlong end = time_clock_us() + timeout; > > > > Could this sum overflow (for example, if the -t option is used)? > > I don't think so: timeout is limited to ZLONG_MAX / 2, which > corresponds to 140000 years or so if zlong is 64-bit. For the sum > to overflow, the current time as given by either gettimeofday() or > clock_gettime(CLOCK_MONOTONIC) would have to also be greater than that. > Sounds good. > OTOH, if zlong is only 32-bit, there could be a problem, and in fact my > function time_clock_us() might not work. However, it was suggested in > message 45250 that it shouldn't be my problem. Should it be? Should > this whole patch be disabled somehow on systems that don't support any > 64-bit type? I think pws was saying there that failure to detect a 64-bit type _when one is available_ isn't your problem; i.e., that you can trust configure to detect a 64-bit type when one is available. How likely is zsh 5.9 to be ported to a platform that doesn't have a 64-bit type? If that's a concern, I'd recommend to issue a warning or disable the feature when we detect that case. I'm not sure whether that's a concern. > > > + if (now + timeout_retry > end) { > > > > Could this sum overflow (for example, if the -i option is used)? > > It might if long and zlong are the same size, as timeout_retry > is limited to LONG_MAX. The program would then wait past the timeout > by one interval. But, again, this interval would have to be hundreds > of thousands of years; if a user requests that, in my opinion, we might > as well just wait forever. Thanks, agreed. > Still, would you like it better if I limited the interval to > min(LONG_MAX, ZLONG_MAX / 2) instead of LONG_MAX? Well, it sounds like that won't make any difference to bin_system_flock()'s behaviour in practice (at least until someone has servers with uptimes on the order of tens of kiloyears), so I don't have a strong preference. I suppose I'd recommend whichever of these is more likely to remain correct even if the code is copied elsewhere and adapted. Thanks, Daniel