From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 444b038c for ; Sun, 5 Jan 2020 18:41:25 +0000 (UTC) Received: (qmail 14636 invoked by alias); 5 Jan 2020 18:41:20 -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: 45244 Received: (qmail 22364 invoked by uid 1010); 5 Jan 2020 18:41:20 -0000 X-Qmail-Scanner-Diagnostics: from mail-yb1-f195.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.1/25684. spamassassin: 3.4.2. Clear:RC:0(209.85.219.195):SA:0(-1.9/5.0):. Processed in 4.188176 secs); 05 Jan 2020 18:41:20 -0000 X-Envelope-From: dana@dana.is X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at _netblocks.google.com designates 209.85.219.195 as permitted sender) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=Ih04IJSswj77YpN4mlRGhBQNbdO6FpB8jQaZUlHU/ac=; b=aB6YIXqfUPl9FA8fT4E+3MLeLD3Odq1vgzYTYdsRbvj1fRj3eMXANKtRSa1XcrgyQd xO8nVhsjnQ2GVuY0MlwfCdLEh6AoqFwBSxJqJsaDKL4Sxx5VTQZxkMGSzyH1VhPql0Tv 7voJXaNyucZhiwhIwJb8SprPGOmCj7bLYbIBG+0IjyGhANMaCMh1Ao9xjzyZMDX9KsqK 37ChoRJ2XMM8PtzWzQULjEVBBxkzKeOHjIgvJ3iVVWEXczoWUMnjd9itu++GcAh3975V l98URNTynVnVaGsbzHT/7wO7Jj1fgkmtO2G0cUmTb8y84T3K/72s9D1z14Doatffj/SB tSrA== X-Gm-Message-State: APjAAAVHma1NwJeGrwtFD8IufiRnDWWI4XNz4dqNXlXMU2LEiS6H9UbX Xnrbh/YZ1mh66gMrwHnJK9lNo+rqcwPmkA== X-Google-Smtp-Source: APXvYqxINw92qr2RS3whqBcpNtTmco1nHjWkO9zLMmDLXhIScPp8u1T3KKMgnp77qxZzMvLYsps4cg== X-Received: by 2002:a25:40c2:: with SMTP id n185mr75175753yba.121.1578249641955; Sun, 05 Jan 2020 10:40:41 -0800 (PST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: Re: [PATCH] Enable sub-second timeout in zsystem flock From: dana In-Reply-To: <20200104184734.ienw42rwq2xu6aap@phare.normalesup.org> Date: Sun, 5 Jan 2020 12:42:50 -0600 Cc: "zsh-workers@zsh.org" Content-Transfer-Encoding: quoted-printable Message-Id: <43775C64-0254-45AB-81AB-B04AE80C4416@dana.is> References: <20190729203521.upp5ku3bsr3hsnxq@phare.normalesup.org> <20200104184734.ienw42rwq2xu6aap@phare.normalesup.org> To: Cedric Ware X-Mailer: Apple Mail (2.3445.104.11) On 4 Jan 2020, at 12:47, Cedric Ware = wrote: > This is a re-send/update of a patch I'd proposed some time ago Feedback off the top of my head (haven't tested and don't know whether = anyone else would object to the change): > +seconds; fractional seconds are allowed. The shell will attempt > +to lock the file every var(period) seconds during this period: > +once a second by default, or according to the value set by option > +tt(-p). The first thing that occurs to me is that you've now decoupled the = polling interval and time-out, which means that not only can you *manually* = specify a polling interval greater than the time-out, it's very possible for the *default* interval to be greater than the time-out. In fact any -t value = less than one second is kind of pointless unless you also give -p I'm not sure how you'd address this functionality-wise. Should it be an = error? idk. It should at least be mentioned in the documentation, though Also this addition is worded a bit strangely i think. period is an = argument to -p; it isn't used unless -p is. Maybe something like: 'During this = period, the shell will attempt to lock the file every period seconds if the -p = option is given, otherwise once a second.' Something to that effect anyway > + long timeout_retry =3D 1e6; > + mnumber timeout_param; It's kind of confusing that these are both named 'timeout' when they = don't specifically have anything to do with the time-out > + zlong timeout_retry_tmp =3D timeout_param.u.d * 1e6; zsh mandates C89, you can't declare variables in the middle of a block > + timeout_retry =3D (timeout_retry_tmp > LONG_MAX) ? > + LONG_MAX : timeout_retry_tmp; Should *this* kind of thing be an error? I guess there's not really any error-checking in the existing option-handling code, though > +time_clock_us(void) Feels like a weird function name, but i don't have a better suggestion btw, it would probably be ideal if shtimer (the variable the shell uses = for the time built-in, the SECONDS parameter, &c.) was monotonic as well. = But that's a separate thing > Is there a more rigorous test framework for this kind of thing, or > anything else to be done that might help this patch be included? The test stuff is in the Test directory in the repo. There don't seem to = be any existing tests for zsystem, so you would have to make like a V13zsystem.ztst (see the other V* files for inspiration). Not sure what = the tests would actually do, though. Maybe you could call it with low -p/-t = values and use SECONDS to ensure that it times out roughly when it's supposed = to? dana