Re: [PATCHv1] [long] improvements to limit/ulimit API and doc

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Good morning Stephane,

Stephane Chazelas wrote on Mon, Nov 23, 2020 at 21:49:42 +0000:
> BTW, POSIX is currently looking at extending the "ulimit"
> builtin specification
> (https://www.austingroupbugs.net/view.php?id=1418) which is what
> got me looking in the first place. I think zsh is mostly
> compliant with their currently proposed resolution.

Thanks for the heads-up.

> I found a small issue in the "limit" builtin in that for instance
> "limit msgqueue 1M" sets that limit to 1 byte instead of 1MiB
> (that M suffix is silently ignored) and that "limit msgqueue 10"
> was not following documentation and tried to have a go at
> addressing it, but ended up finding a few more small issues,
> went down the rabbit hole, also trying to address the problem
> (not specific to zsh) that you never know what unit those
> resource limits are meant to be expressed as.

> From 5c1971d40b238edb1a745b7298b0169cff2b8053 Mon Sep 17 00:00:00 2001
> From: Stephane Chazelas <stephane@xxxxxxxxxxxx>
> Date: Mon, 23 Nov 2020 17:31:02 +0000
> Subject: [PATCH] improve limit/ulimit API and documentation
> 
> A few issues addressed:
> 

This patch is over 1k lines long and makes multiple independent changes.

Could you please split this into one patch per logical change?  That
would make it easier to review, both now, and in the future should
a regression be bisected to it.

Thanks,

Daniel

> * msgqueue limit specifies a number of bytes but was classified as
>   ZLIMTYPE_NUMBER which meant k/m suffixes were not allowed and the
>   default unit for `limit` was 1 (byte) instead of the 1024 (KiB)
>   specified by documentation.
> 
>   -> turns out tcsh's `limit` handled that limit (called `maxmessage`
>      there) the same way and `bash`, `bosh` and `mksh`'s `ulimit` (not
>      ksh93 which takes kibibytes there) also expect a number of bytes
>      there.
> 
>      So, the type was changed to ZLIMTYPE_MEMORY, but the unit remains
>      bytes for both `limit` and `ulimit` as a (now documented) special
>      quirk for backward compatibility.
> 
> * unit suffixes, where not supported (like for ZLIMTYPE_NUMBER) are
>   silently ignored.
> 
>   -> now return an error on unexpected or unrecognised suffixes.
> 
> * limit output using ambigous kB, MB units. These days, those tend to
>   imply decimal scaling factors (1000B, 1000000B).
> 
>   -> changed output to use KiB, MiB, the ISO 80000 unambiguous ones
>      which are now more or less mainstream.
>   -> those, extended to KMGTPE..iB are also accepted on input while
>      KB, MB... are interpreted as decimal. (K, M, G remain binary).
>   -> documentation updated to avoid kilobyte, megabyte and use
>      unambiguous kibibyte, mebibyte... instead.
> 
> -> rt_time limit is now `ulimit -R` like in bash or bosh.
> 
> * "nice" limit description ("max nice") was misleading as it's more
>   linked to the *minimum* niceness the process can get.
> 
>   -> Changed to "max nice priority" matching the Linux kernel's own
>      description.
> 
> * documentation for both `limit` and `ulimit` missing a few limits ->
>   added
> 
> * time limits are output as h:mm:ss but that's not accepted on input.
>   1:00:00 silently truncated to 1:00 (one minute instead of one hour).
> 
>   -> accept [[hh:]mm:]ss[.usec]
> 
> * limit and ulimit output truncate precision (1000B limit represented as
>   0kB and 1 respectively)
> 
>   -> addressed in `limit` but not `ulimit` (where
>      compliance/compatibility with ksh matters). By only using scaling
>      factors when the limit can be represented in an integer number of
>      them (using B, as in 1000B above when none qualify).
> 
> * some limits can't be set to arbitrary values (like that 1000 above for
>   filesize) and it's not always obvious what the default unit should be.
> 
>   -> recognise the B suffix on input for "memory" type limits and
>      "s"/"ms"/"ns" for time/microsecond type units so the user can input
>      values unambiguously.
>   -> those suffixes are now recognised by both limit and ulimit. Parsing
>      code factorised into `zstrtorlimt`.
> 
> * `limit` and `unlimit` are disabled when zsh is started in csh
>   emulation even though those builtins come from there.
> 
>   -> changed the features_emu in rlimits.mdd (only used there) and
>     mkbltnmlst.sh to features_posix for those to only be disabled
>     in sh/ksh emulation.
> 
> * `limit` changes the limits for children, `ulimit` for the shell,
>   but both report limits for children, and it's not possible to
>   retrieve the shell's own limits.
> 
>   -> ulimit not changed as it's probably better that way.
>   -> `limit -s somelimit` changed so it reports the somelimit value
>      for the shell process
> 
> -> documentation improved.
> ---
>  Doc/Zsh/builtins.yo      | 129 +++++++++----
>  Doc/Zsh/expn.yo          |  18 +-
>  Doc/Zsh/mod_zpty.yo      |   4 +-
>  Doc/Zsh/params.yo        |  10 +-
>  NEWS                     |   7 +
>  Src/Builtins/rlimits.c   | 390 +++++++++++++++++++++++++++------------
>  Src/Builtins/rlimits.mdd |   8 +-
>  Src/mkbltnmlst.sh        |  10 +-
>  Test/B12limit.ztst       | 129 +++++++++++++
>  9 files changed, 530 insertions(+), 175 deletions(-)
> 
> Though I think it could be applied as is (I don't think I've
> broken backward compatibility), there are a few things I'm not
> completely happy or sure about so I'd appreciate some input.
> 
> The few remaining issues I've not really addressed here:
> 
> - ulimit output rounds down the values (some of them anyway) so
>   we lose information. Is it worth addressing? (like with a
>   "raw" option)?
> 
> - Some might disapprove the switch to kibibyte/mebibyte KiK/MiB
>   (and the MB meaning 1,000,000).
> 
> - Is it worth accepting floating point values like:
>   limit filesize 1.2G
> 
> - I've only tested it on Ubuntu, FreeBSD and Cygwin. I suspect
>   my test cases will fail on 32bit systems. They're skipped on
>   Cygwin which doesn't let you set any limit AFAICT. I don't
>   have much coverage in those two tests, but with limits being
>   very system specific, I'm not sure how to tackle it.
> 
> - ulimit still outputs the limits set for children processes. I
>   think that's probably best. So it outputs the limits set by
>   limit, ulimit or ulimit -s, even if strictly speaking, it
>   doesn't give you what's returned by getrlimit() like in other
>   shells (that only ever becomes visible if you use "limit"
>   anyway which is not in those other shells.
> 
> - there are some corner case issues that could surprise users
>   and may be worth documenting like:
>   (limit filesize 1k; (print {1..2000} > file)) still creates a
>   8K file because the fork for the (print...) was optimised out
>   so the limits are not applied.
> 
> - I've made it so "limit -s filesize" reports the shell's own
>   limit (-s is for "set" initially, but it could also be "shell"
>   or "self"). But while "limit" outputs all children limits,
>   "limit -s" still copies those children limits to the shell's
>   and there's no way to print *all* self limits. That doesn't
>   make for a very intuitive API.