Re: [PATCH] Enable sub-second timeout in zsystem flock

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

dana wrote on Tue, 14 Apr 2020 21:05 -0500:
> On 14 Apr 2020, at 20:15, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > dana, would you happen to have time to review the Cedric's latest
> > revision (workers/45690)?  
> 
> I'm not sure i fully understand the implications of the 32-vs-64-bit stuff,
> nor whether it matters enough to go to all the extra trouble.

It's a theory v. practice issue.

In theory, zsh is written in C89, and C89 doesn't guarantee a 64-bit
integer type.

In practice, C89 is 31 years old; C99 does guarantee a 64-bit type; and
we don't positively know of anyone porting zsh to a platform that
doesn't have a 64-bit type; so the concern may well be academic, or
nearly so.

> But aside from that and the two bits you mentioned (agreed on the
> semicolon), the only thing i noticed is that there are several
> instances where the formatting of the new C code doesn't match its
> surroundings, e.g. unnecessary brackets in ((foo < bar) || (baz > qux))

I don't mind these.  They add clarity and don't hurt readability.

> and inconsistent white space in foo? bar : baz

Agreed.  (I noticed them too, but didn't want to nitpick from the peanut
gallery.)

In other news, how about using real superscripts:

--- a/Doc/Zsh/mod_system.yo
+++ b/Doc/Zsh/mod_system.yo
@@ -203,7 +203,7 @@ if the tt(-i) var(interval) option is given, otherwise once a second.
 so that the shell waits only until the var(timeout) and not longer.)
 If the attempt times out, status 2 is returned.

-(Note: var(timeout) must be less than 2^30-1 seconds (about 34 years);
+(Note: var(timeout) must be less than ifzman(2^30-1)ifnzman(2NOTRANS(@sup{30})-1) seconds (about 34 years)
 and var(interval) must be less than 0.999 * LONG_MAX microseconds
 (only about 35 minutes on 32-bit systems).)

(To see the difference, «make -C Doc html pdf».)

> Otherwise i think it makes sense, as far as i understand it, and the tests
> pass on my machine. I don't see any functional issues

If you think the feature makes sense and the implementation is robust,
why not merge it (once the 64-bit question is settled)?  Between you and
Cedric there have already been two pairs of eyes on the code, after all.

Cheers,

Daniel