zsh-workers
 help / color / mirror / Atom feed
* [PATCHv1] [long] improvements to limit/ulimit API and doc
@ 2020-11-23 21:49 Stephane Chazelas
  2020-11-25  0:35 ` Daniel Shahaf
                   ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-23 21:49 UTC (permalink / raw)
  To: Zsh hackers list

Hello,

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.

Please see the details in commit log below.

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.

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.


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

A few issues addressed:

* 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(-)

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index ebb29f632..f794abf1a 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1116,15 +1116,18 @@ cindex(resource limits)
 cindex(limits, resource)
 item(tt(limit) [ tt(-hs) ] [ var(resource) [ var(limit) ] ] ...)(
 Set or display resource limits.  Unless the tt(-s) flag is given,
-the limit applies only the children of the shell.  If tt(-s) is
-given without other arguments, the resource limits of the current
-shell is set to the previously set resource limits of the children.
+the limit applies only to child processes and executed commands, not the
+current shell process itself.
 
-If var(limit) is not specified, print the current limit placed
-on var(resource), otherwise
-set the limit to the specified value.  If the tt(-h) flag
-is given, use hard limits instead of soft limits.
-If no var(resource) is given, print all limits.
+If tt(-s) is given without other arguments, the resource limits of the
+current shell is set to the previously set resource limits of the
+children.
+
+If var(limit) is not specified, print the current limit placed on
+var(resource) (of the shell with tt(-s), of children without), otherwise
+set the limit to the specified value.  If the tt(-h) flag is given, use
+hard limits instead of soft limits. If no var(resource) is given, print
+all limits.
 
 When looping over multiple resources, the shell will abort immediately if
 it detects a badly formed argument.  However, if it fails to set a limit
@@ -1143,18 +1146,23 @@ sitem(tt(datasize))(Maximum data size (including stack) for each process.)
 sitem(tt(descriptors))(Maximum value for a file descriptor.)
 sitem(tt(filesize))(Largest single file allowed.)
 sitem(tt(kqueues))(Maximum number of kqueues allocated.)
+sitem(tt(maxfilelocks))(Maximum number of file locks.)
 sitem(tt(maxproc))(Maximum number of processes.)
-sitem(tt(maxpthreads))(Maximum number of threads per process.)
+sitem(tt(maxpthreads))(Maximum number of threads per uid (NetBSD/OpenBSD) or per process.)
 sitem(tt(memorylocked))(Maximum amount of memory locked in RAM.)
 sitem(tt(memoryuse))(Maximum resident set size.)
 sitem(tt(msgqueue))(Maximum number of bytes in POSIX message queues.)
+sitem(tt(nice))(Maximum nice priority.)
 sitem(tt(posixlocks))(Maximum number of POSIX locks per user.)
 sitem(tt(pseudoterminals))(Maximum number of pseudo-terminals.)
 sitem(tt(resident))(Maximum resident set size.)
+sitem(tt(rt_priority))(Maximum realtime priority.)
+sitem(tt(rt_time))(Maximum realtime CPU time slice.)
 sitem(tt(sigpending))(Maximum number of pending signals.)
 sitem(tt(sockbufsize))(Maximum size of all socket buffers.)
 sitem(tt(stacksize))(Maximum stack size for each process.)
 sitem(tt(swapsize))(Maximum amount of swap used.)
+sitem(tt(umtxp))(Maximum number of umtx shared locks.)
 sitem(tt(vmemorysize))(Maximum amount of virtual memory.)
 endsitem()
 
@@ -1169,18 +1177,46 @@ the limit anyway, and will report an error if this fails.  As the shell
 does not store such resources internally, an attempt to set the limit will
 fail unless the tt(-s) option is present.
 
-var(limit) is a number, with an optional scaling factor, as follows:
+If var(limit) is a decimal integer number without suffix, then for
+historical reason and compatibility with csh where that command comes from,
+the unit will depend on the type of limit: microsecond for tt(rt_time),
+second for all other time limits, kibibytes (1024 bytes) for all limits that
+express a number of bytes (except tt(msgqueue)).
+
+Instead, to avoid confusion, a suffix (case insensitive) may be appended to
+the (integer only) decimal number to specify the unit:
 
 startsitem()
-sitem(var(n)tt(h))(hours)
-sitem(var(n)tt(k))(kilobytes (default))
-sitem(var(n)tt(m))(megabytes or minutes)
-sitem(var(n)tt(g))(gigabytes)
-sitem([var(mm)tt(:)]var(ss))(minutes and seconds)
+sitem(time limits)(
+startsitem()
+sitem(tt(h))(hours)
+sitem(tt(m))(minutes)
+sitem(tt(s))(seconds)
+sitem(tt(ms))(milliseconds)
+sitem(tt(us))(microseconds)
+sitem(tt([[var(h):]var(m):]var(s)[.var(usec)]))(all combined)
+endsitem()
+
+For instance a 2 millisecond limit of tt(rt_time), can be expressed as
+tt(2ms), tt(2000us), tt(0.002), tt(0:0:0.002) or tt(2000) without unit.)
+sitem(memory limits)(
+startsitem()
+sitem(tt(k, kib))(kibibytes (1024 bytes))
+sitem(tt(m, mib))(mibibytes)
+sitem(...)(and so on with tt(g), tt(t), tt(p), tt(e) for gibibytes,
+tebibytes, pebibytes and exbibytes respectively)
+sitem(tt(kb))(kilobytes (1,000 bytes))
+sitem(tt(mb))(megabytes (1,000,000 bytes))
+sitem(...)(and so on with tt(g), tt(t), tt(p), tt(e) for gigabytes,
+terabytes, petabytes and exabytes respectively.)
+sitem(tt(b))(em(byte) to set the limit with arbitrary precisions)
+endsitem())
+sitem(other limits)(other limits are assumed to be numerical and ony
+a decimal integer number is accepted.)
 endsitem()
 
 The tt(limit) command is not made available by default when the
-shell starts in a mode emulating another shell.  It can be made available
+shell starts in sh or ksh emulation mode.  It can be made available
 with the command `tt(zmodload -F zsh/rlimits b:limit)'.
 )
 findex(local)
@@ -2250,8 +2286,8 @@ together with the tt(-H) flag set both hard and soft limits.
 If no options are used, the file size limit (tt(-f)) is assumed.
 
 If var(limit) is omitted the current value of the specified resources are
-printed.  When more than one resource value is printed, the limit name and
-unit is printed before each value.
+printed (rounded down to the corresponding unit).  When more than one resource
+value is printed, the limit name and unit is printed before each value.
 
 When looping over multiple resources, the shell will abort immediately if
 it detects a badly formed argument.  However, if it fails to set a limit
@@ -2260,30 +2296,45 @@ for some other reason it will continue trying to set the remaining limits.
 Not all the following resources are supported on all systems.  Running
 tt(ulimit -a) will show which are supported.
 
+Whilst tt(limit) is the BSD/csh-style command, tt(ulimit) is the SysV/Korn
+shell equivalent (added later to zsh for compatibility with ksh). While
+tt(limit) in zsh sets the limits for child processes by default, tt(ulimit)
+sets them for both the shell and child processes, and reports the ones
+set for child processes.
+
+The same suffixes are recognised as for tt(limit), but bear in mind that
+default units when no suffix is specified varry between the two.
+
+Below, the corresponding tt(limit) keyword for each tt(ulimit) option is
+shown in parenthesis for reference:
+
 startsitem()
 sitem(tt(-a))(Lists all of the current resource limits.)
-sitem(tt(-b))(Socket buffer size in bytes LPAR()N.B. not kilobytes+RPAR())
-sitem(tt(-c))(512-byte blocks on the size of core dumps.)
-sitem(tt(-d))(Kilobytes on the size of the data segment.)
-sitem(tt(-f))(512-byte blocks on the size of files written.)
-sitem(tt(-i))(The number of pending signals.)
-sitem(tt(-k))(The number of kqueues allocated.)
-sitem(tt(-l))(Kilobytes on the size of locked-in memory.)
-sitem(tt(-m))(Kilobytes on the size of physical memory.)
-sitem(tt(-n))(open file descriptors.)
-sitem(tt(-p))(The number of pseudo-terminals.)
-sitem(tt(-q))(Bytes in POSIX message queues.)
+sitem(tt(-b))(Socket buffer size in bytes LPAR()N.B. not kibibytes+RPAR() (tt(sockbufsize)).)
+sitem(tt(-c))(512-byte blocks on the size of core dumps (tt(coredumpsize)).)
+sitem(tt(-d))(Kibibytes on the size of the data segment (tt(datasize)).)
+sitem(tt(-e))(nice priority (tt(nice)).)
+sitem(tt(-f))(512-byte blocks on the size of files written (tt(filesize)).)
+sitem(tt(-i))(The number of pending signals (tt(sigpending)).)
+sitem(tt(-k))(The number of kqueues allocated (tt(kqueues)).)
+sitem(tt(-l))(Kibibytes on the size of locked-in memory (tt(memorylocked)).)
+sitem(tt(-m))(Kibibytes on the size of physical memory (tt(resident)).)
+sitem(tt(-n))(open file descriptors (tt(descriptors)).)
+sitem(tt(-o))(umtx shared locks (tt(umtxp)).)
+sitem(tt(-p))(The number of pseudo-terminals (tt(pseudoterminals)).)
+sitem(tt(-q))(Bytes in POSIX message queues (tt(msgqueue)).)
 sitem(tt(-r))(Maximum real time priority.  On some systems where this
 is not available, such as NetBSD, this has the same effect as tt(-T)
-for compatibility with tt(sh).)
-sitem(tt(-s))(Kilobytes on the size of the stack.)
-sitem(tt(-T))(The number of simultaneous threads available to the user.)
-sitem(tt(-t))(CPU seconds to be used.)
-sitem(tt(-u))(The number of processes available to the user.)
-sitem(tt(-v))(Kilobytes on the size of virtual memory.  On some systems this
-refers to the limit called `address space'.)
-sitem(tt(-w))(Kilobytes on the size of swapped out memory.)
-sitem(tt(-x))(The number of locks on files.)
+for compatibility with tt(sh) (tt(rt_priority) / tt(maxpthreads)).)
+sitem(tt(-R))(realtime CPU time slice (tt(rt_time)).)
+sitem(tt(-s))(Kibibytes on the size of the stack (tt(stacksize)).)
+sitem(tt(-T))(The number of simultaneous threads available to the user (tt(maxpthreads)).)
+sitem(tt(-t))(CPU seconds to be used (tt(cputime)).)
+sitem(tt(-u))(The number of processes available to the user (tt(maxproc)).)
+sitem(tt(-v))(Kibibytes on the size of virtual memory.  On some systems this
+refers to the limit called em(address space) (tt(vmemorysize) / tt(addressspace)).)
+sitem(tt(-w))(Kibibytes on the size of swapped out memory (tt(swapsize)).)
+sitem(tt(-x))(The number of locks on files (tt(maxfilelocks)).)
 endsitem()
 
 A resource may also be specified by integer in the form `tt(-N)
@@ -2343,7 +2394,7 @@ The resources of the shell process are only changed if the tt(-s)
 flag is given.
 
 The tt(unlimit) command is not made available by default when the
-shell starts in a mode emulating another shell.  It can be made available
+shell starts in sh or ksh emulation mode.  It can be made available
 with the command `tt(zmodload -F zsh/rlimits b:unlimit)'.
 )
 findex(unset)
diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index b3396721f..588426af0 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -2837,15 +2837,15 @@ exactly var(n) bytes in length.
 
 If this flag is directly followed by a em(size specifier) `tt(k)' (`tt(K)'),
 `tt(m)' (`tt(M)'), or `tt(p)' (`tt(P)') (e.g. `tt(Lk-50)') the check is
-performed with kilobytes, megabytes, or blocks (of 512 bytes) instead.
-(On some systems additional specifiers are available for gigabytes,
-`tt(g)' or `tt(G)', and terabytes, `tt(t)' or `tt(T)'.) If a size specifier
-is used a file is regarded as "exactly" the size if the file size rounded up
-to the next unit is equal to the test size.  Hence `tt(*LPAR()Lm1+RPAR())'
-matches files from 1 byte up to 1 Megabyte inclusive.  Note also that
-the set of files "less than" the test size only includes files that would
-not match the equality test; hence `tt(*LPAR()Lm-1+RPAR())' only matches
-files of zero size.
+performed with kibibytes, mebibytes, or blocks (of 512 bytes; catch: not
+em(pebibytes)) instead. (On some systems additional specifiers are available
+for gibibytes, `tt(g)' or `tt(G)', and tebibytes, `tt(t)' or `tt(T)'.) If a
+size specifier is used, a file is regarded as "exactly" the size if the file
+size rounded up to the next unit is equal to the test size.  Hence
+`tt(*LPAR()Lm1+RPAR())' matches files from 1 byte up to 1 Mebibyte
+inclusive.  Note also that the set of files "less than" the test size only
+includes files that would not match the equality test; hence
+`tt(*LPAR()Lm-1+RPAR())' only matches files of zero size.
 )
 item(tt(^))(
 negates all qualifiers following it
diff --git a/Doc/Zsh/mod_zpty.yo b/Doc/Zsh/mod_zpty.yo
index 3ca031c01..ba59e1783 100644
--- a/Doc/Zsh/mod_zpty.yo
+++ b/Doc/Zsh/mod_zpty.yo
@@ -66,8 +66,8 @@ read matches the var(pattern), even in the non-blocking case.  The return
 status is zero if the string read matches the pattern, or if the command
 has exited but at least one character could still be read.  If the option
 tt(-m) is present, the return status is zero only if the pattern matches.
-As of this writing, a maximum of one megabyte of output can be consumed
-this way; if a full megabyte is read without matching the pattern, the
+As of this writing, a maximum of one mebibyte of output can be consumed
+this way; if a full mebibyte is read without matching the pattern, the
 return status is non-zero.
 
 In all cases, the return status is non-zero if nothing could be read, and
diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index 36c1ae4c2..6c305da34 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -1473,7 +1473,7 @@ is specified with no command.  Defaults to tt(more).
 vindex(REPORTMEMORY)
 item(tt(REPORTMEMORY))(
 If nonnegative, commands whose maximum resident set size (roughly
-speaking, main memory usage) in kilobytes is greater than this
+speaking, main memory usage) in kibibytes is greater than this
 value have timing statistics reported.  The format used to output
 statistics is the value of the tt(TIMEFMT) parameter, which is the same
 as for the tt(REPORTTIME) variable and the tt(time) builtin; note that
@@ -1603,12 +1603,12 @@ sitem(tt(%E))(Elapsed time in seconds.)
 sitem(tt(%P))(The CPU percentage, computed as
 100*(tt(%U)PLUS()tt(%S))/tt(%E).)
 sitem(tt(%W))(Number of times the process was swapped.)
-sitem(tt(%X))(The average amount in (shared) text space used in kilobytes.)
+sitem(tt(%X))(The average amount in (shared) text space used in kibibytes.)
 sitem(tt(%D))(The average amount in (unshared) data/stack space used in
-kilobytes.)
-sitem(tt(%K))(The total space used (tt(%X)PLUS()tt(%D)) in kilobytes.)
+kibibytes.)
+sitem(tt(%K))(The total space used (tt(%X)PLUS()tt(%D)) in kibibytes.)
 sitem(tt(%M))(The  maximum memory the process had in use at any time in
-kilobytes.)
+kibibytes.)
 sitem(tt(%F))(The number of major page faults (page needed to be brought
 from disk).)
 sitem(tt(%R))(The number of minor page faults.)
diff --git a/NEWS b/NEWS
index a8e7df80e..f5534eba0 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,13 @@ The zsh/system module's `zsystem flock` command learnt an -i option to
 set the wait interval used with -t. Additionally, -t now supports
 fractional seconds.
 
+The `limit` builtin accepts more units for resource limits, now shared
+with the `ulimit` builtin allowing to specify arbitrary limit values.
+`limit -s somelimit` now reports the shell process' own value of the limit.
+The `limit` and `unlimit` builtins are now available again in csh emulation.
+More generally, the resouce limit handling interfaces and documentation have
+been improved.
+
 Changes from 5.7.1-test-3 to 5.8
 --------------------------------
 
diff --git a/Src/Builtins/rlimits.c b/Src/Builtins/rlimits.c
index 5f9c84b0f..1b2474583 100644
--- a/Src/Builtins/rlimits.c
+++ b/Src/Builtins/rlimits.c
@@ -55,7 +55,8 @@ typedef struct resinfo_T {
  * 1. Add zsh_LIMIT_PRESENT(RLIMIT_XXX) in configure.ac.
  * 2. Add an entry for RLIMIT_XXX to known_resources[].
  *    Make sure the option letter (resinto_T.opt) is unique.
- * 3. Build zsh and run the test B12rlimit.ztst.
+ * 3. Add entry in documentation for the limit and ulimit builtins
+ * 4. Build zsh and run the test B12rlimit.ztst.
  */
 static const resinfo_T known_resources[] = {
     {RLIMIT_CPU, "cputime", ZLIMTYPE_TIME, 1,
@@ -109,12 +110,23 @@ static const resinfo_T known_resources[] = {
 		'i', "pending signals"},
 # endif
 # ifdef HAVE_RLIMIT_MSGQUEUE
-    {RLIMIT_MSGQUEUE, "msgqueue", ZLIMTYPE_NUMBER, 1,
+    {RLIMIT_MSGQUEUE, "msgqueue", ZLIMTYPE_MEMORY, 1,
 		'q', "bytes in POSIX msg queues"},
 # endif
 # ifdef HAVE_RLIMIT_NICE
+    /*
+     * Not max niceness. On Linux ranges [1..40] for that RLIM translates
+     * to [-19..20] meaning that for instance setting it to 20 means
+     * processes can lower their niceness as far down as -1 and with the
+     * default of 0, unprivileged processes can't lower their niceness.
+     *
+     * Increasing niceness is always possible.
+     *
+     * "max nice priority" is the wording used in /proc/self/limits on
+     * Linux.
+     */
     {RLIMIT_NICE, "nice", ZLIMTYPE_NUMBER, 1,
-		'e', "max nice"},
+		'e', "max nice priority"},
 # endif
 # ifdef HAVE_RLIMIT_RTPRIO
     {RLIMIT_RTPRIO, "rt_priority", ZLIMTYPE_NUMBER, 1,
@@ -122,7 +134,7 @@ static const resinfo_T known_resources[] = {
 # endif
 # ifdef HAVE_RLIMIT_RTTIME
     {RLIMIT_RTTIME, "rt_time", ZLIMTYPE_MICROSECONDS, 1,
-		'N', "rt cpu time (microseconds)"},
+		'R', "rt cpu time slice (microseconds)"},
 # endif
     /* BSD */
 # ifdef HAVE_RLIMIT_SBSIZE
@@ -192,9 +204,9 @@ set_resinfo(void)
 	if (!resinfo[i]) {
 	    /* unknown resource */
 	    resinfo_T *info = (resinfo_T *)zshcalloc(sizeof(resinfo_T));
-	    char *buf = (char *)zalloc(12);
+	    char *buf = (char *)zalloc(8 /* UNKNOWN- */ + 3 /* digits */ + 1 /* '\0' */);
 	    snprintf(buf, 12, "UNKNOWN-%d", i);
-	    info->res = - 1;	/* negative value indicates "unknown" */
+	    info->res = -1;	/* negative value indicates "unknown" */
 	    info->name = buf;
 	    info->type = ZLIMTYPE_UNKNOWN;
 	    info->unit = 1;
@@ -255,38 +267,204 @@ printrlim(rlim_t val, const char *unit)
 # endif /* RLIM_T_IS_QUAD_T */
 }
 
+/*
+ * Parse a string into the corresponding value based on the limit type
+ *
+ *   ZLIMTYPE_UNKNOWN / ZLIMTYPE_NUMBER:
+ *     decimal integer without sign only: raw value
+ *
+ *   ZLIMTYPE_TIME / ZLIMTYPE_MICROSECONDS:
+ *     <decimal> only gives seconds or microseconds depending on type
+ *     or:
+ *     [[hour:]min:]sec[.usec]
+ *     or:
+ *     <decimal> with h (hour), m (min), s (sec), ms, us suffix.
+ *
+ *   ZLIMTYPE_MEMORY:
+ *     <decimal> without suffix interpreted as KiB by "limit" (except for
+ *     RLIMIT_MSGQUEUE, see below) and based on resinfo.unit by "ulimit".
+ *
+ *     K/M/G/T/P/E suffix and same with iB suffix use 1024 factor
+ *     KB/MB/GB... use 1000 factor.
+ *
+ *     B for bytes (avoids that mess about default units).
+ *
+ * All suffixes are case insensitive.
+ *
+ */
+
 /**/
 static rlim_t
-zstrtorlimt(const char *s, char **t, int base)
+zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 {
     rlim_t ret = 0;
+    const char *orig = s;
+    enum zlimtype type = resinfo[lim]->type;
+    *err = NULL;
 
-    if (strcmp(s, "unlimited") == 0) {
-	if (t)
-	    *t = (char *) s + 9;
+    if (strcmp(s, "unlimited") == 0)
 	return RLIM_INFINITY;
+
+    for (; *s >= '0' && *s <= '9'; s++)
+	ret = ret * 10 + *s - '0';
+
+    if (s == orig) {
+	*err = "decimal integer expected";
+	return 0;
+    }
+
+    if (lim >= RLIM_NLIMITS ||
+	type == ZLIMTYPE_NUMBER ||
+	type == ZLIMTYPE_UNKNOWN) {
+	/*
+	 * pure numeric resource -- only a straight decimal number is
+	 * permitted.
+	 */
+	if (*s) {
+	    *err = "limit must be a decimal integer";
+	    return 0;
+	}
+    }
+    else if (type == ZLIMTYPE_TIME ||
+	     type == ZLIMTYPE_MICROSECONDS) {
+	if (*s) {
+	    int divisor = 1;
+	    int factor = 1;
+	    switch (*s++) {
+
+	    case 'm': /* m for minute or ms for milisecond */
+	    case 'M':
+		if (*s == 's' || *s == 'S') {
+		    s++;
+		    divisor = 1000;
+		}
+		else {
+		    factor = 60;
+		}
+		break;
+
+	    case 'h':
+	    case 'H':
+		factor = 3600;
+		break;
+
+	    case 's':
+	    case 'S':
+		break;
+
+	    case 'u':
+	    case 'U':
+		divisor = 1000000;
+		if (*s == 's' || *s == 'S')
+		    s++;
+		break;
+
+	    case ':':
+		do {
+		    int more = 0;
+		    while (*s >= '0' && *s <= '9') {
+			more = more * 10 + (*s - '0');
+			s++;
+		    }
+		    ret = ret * 60 + more;
+		} while (*s == ':' && ++s);
+		if (*s == '.')
+		    s++;
+		    /* fallthrough */
+		else
+		    break;
+	    case '.':
+		if (type == ZLIMTYPE_MICROSECONDS) {
+		    int frac;
+		    for (frac = 0; *s >= '0' && *s <= '9'; s++) {
+			if (divisor < 1000000) {
+			    /* ignore digits past the 6th */
+			    divisor *= 10;
+			    frac = frac * 10 + (*s - '0');
+			}
+		    }
+		    ret = ret * divisor + frac;
+		}
+		else {
+		    /* fractional part ignored */
+		    while (*s >= '0' && *s <= '9')
+			s++;
+		}
+		break;
+	    default:
+		*err = "invalid time specification";
+		return 0;
+	    }
+
+	    if (*s) {
+		*err = "invalid time specification";
+		return 0;
+	    }
+
+	    ret *= factor;
+	    if (type == ZLIMTYPE_MICROSECONDS)
+		ret *= 1000000 / divisor;
+	    else
+		ret /= divisor;
+	}
+    }
+    else {
+	/*
+	 * memory-type resource
+	 */
+	if (*s) {
+	    if (*s == 'b' || *s == 'B')
+		s++;
+	    else {
+		const char *suffix = "kKmMgGtTpPeE";
+		char *offset;
+
+		if ((offset = strchr(suffix, *s))) {
+		    s++;
+		    if (*s == 'b' || *s == 'B') {
+			/* KB == 1000 */
+			const char *p;
+			for (p = suffix; p <= offset; p += 2)
+			    ret *= 1000;
+			s++;
+		    }
+		    else {
+			/* K/KiB == 1024 */
+			if ((s[0] == 'i' || s[0] == 'I') &&
+			    (s[1] == 'b' || s[1] == 'B'))
+			    s += 2;
+			ret <<= ((offset - suffix) / 2 + 1) * 10;
+		    }
+		}
+	    }
+	    if (*s) {
+		*err = "invalid unit";
+		return 0;
+	    }
+	}
+	else {
+	    if (ulimit)
+		ret *= resinfo[lim]->unit;
+	    else
+#ifdef HAVE_RLIMIT_MSGQUEUE
+		if (lim != RLIMIT_MSGQUEUE)
+		    /*
+		     * Historical quirk. In tcsh's limit (and bash's and mksh's
+		     * ulimit, but not ksh93), that limit expects bytes instead
+		     * of kibibytes and earlier versions of zsh were treating
+		     * it as a ZLIMTYPE_NUMBER.
+		     *
+		     * We still want to treat it as ZLIMTYPE_MEMORY and accept
+		     * KMG... suffixes as it is a number of bytes.
+		     *
+		     * But we carry on taking the value as a number of *bytes*
+		     * in the "limit" builtin for backward compatibility and
+		     * compatibility with tcsh.
+		     */
+#endif
+		    ret *= 1024;
+	}
     }
-# if defined(RLIM_T_IS_QUAD_T) || defined(RLIM_T_IS_LONG_LONG) || defined(RLIM_T_IS_UNSIGNED)
-    if (!base) {
-	if (*s != '0')
-	    base = 10;
-	else if (*++s == 'x' || *s == 'X')
-	    base = 16, s++;
-	else
-	    base = 8;
-    } 
-    if (base <= 10)
-	for (; *s >= '0' && *s < ('0' + base); s++)
-	    ret = ret * base + *s - '0';
-    else
-	for (; idigit(*s) || (*s >= 'a' && *s < ('a' + base - 10))
-	     || (*s >= 'A' && *s < ('A' + base - 10)); s++)
-	    ret = ret * base + (idigit(*s) ? (*s - '0') : (*s & 0x1f) + 9);
-    if (t)
-	*t = (char *)s;
-# else /* !RLIM_T_IS_QUAD_T && !RLIM_T_IS_LONG_LONG && !RLIM_T_IS_UNSIGNED */
-    ret = zstrtol(s, t, base);
-# endif /* !RLIM_T_IS_QUAD_T && !RLIM_T_IS_LONG_LONG && !RLIM_T_IS_UNSIGNED */
     return ret;
 }
 
@@ -317,25 +495,42 @@ showlimitvalue(int lim, rlim_t val)
 	       resinfo[lim]->type == ZLIMTYPE_UNKNOWN)
 	printrlim(val, "\n");	/* pure numeric resource */
     else {
-	/* memory resource -- display with `k' or `M' modifier */
-	if (val >= 1024L * 1024L)
-	    printrlim(val/(1024L * 1024L), "MB\n");
+	/*
+	 * memory resource -- display with KiB/MiB... for exact
+	 * multiples of those units
+	 */
+	const char *units = "KMGTPE";
+	rlim_t v = val;
+	int n = 0;
+	while (units[n] && (v & 1023) == 0 && v >> 10) {
+	    n++;
+	    v >>= 10;
+	}
+	if (n) {
+	    char suffix[] = "XiB\n";
+	    *suffix = units[n-1];
+	    printrlim(v, suffix);
+	}
 	else
-	    printrlim(val/1024L, "kB\n");
+	    printrlim(val, "B\n");
     }
 }
 
-/* Display resource limits.  hard indicates whether `hard' or `soft'  *
- * limits should be displayed.  lim specifies the limit, or may be -1 *
- * to show all.                                                       */
+/*
+ * Display resource limits.  hard indicates whether `hard' or `soft'
+ * limits should be displayed.  lim specifies the limit, or may be -1
+ * to show all.  If `shell' is non-zero, the limits in place for the
+ * shell process retrieved with getrlimits() are shown.
+ */
 
 /**/
 static int
-showlimits(char *nam, int hard, int lim)
+showlimits(char *nam, int hard, int lim, int shell)
 {
     int rt;
+    int ret = 0;
 
-    if (lim >= RLIM_NLIMITS)
+    if (shell || lim >= RLIM_NLIMITS)
     {
 	/*
 	 * Not configured into the shell.  Ask the OS
@@ -357,17 +552,33 @@ showlimits(char *nam, int hard, int lim)
     else
     {
 	/* main loop over resource types */
-	for (rt = 0; rt != RLIM_NLIMITS; rt++)
-	    showlimitvalue(rt, (hard) ? limits[rt].rlim_max :
-			   limits[rt].rlim_cur);
+	for (rt = 0; rt != RLIM_NLIMITS; rt++) {
+	    struct rlimit vals, *plim;
+	    if (shell) {
+		plim = &vals;
+		if (getrlimit(rt, plim) < 0)
+		{
+		    zwarnnam(nam, "can't read \"%s\" limit: %e", resinfo[rt]->name, errno);
+		    ret = 1;
+		    continue;
+		} 
+	    }
+	    else
+		plim = &(limits[rt]);
+
+	    showlimitvalue(rt, (hard) ? plim->rlim_max :
+			   plim->rlim_cur);
+	}
     }
 
-    return 0;
+    return ret;
 }
 
-/* Display a resource limit, in ulimit style.  lim specifies which   *
- * limit should be displayed, and hard indicates whether the hard or *
- * soft limit should be displayed.                                   */
+/*
+ * Display a resource limit, in ulimit style.  lim specifies which
+ * limit should be displayed, and hard indicates whether the hard or
+ * soft limit should be displayed.
+ */
 
 /**/
 static int
@@ -394,12 +605,12 @@ printulimit(char *nam, int lim, int hard, int head)
 	if (lim < RLIM_NLIMITS) {
 	    const resinfo_T *info = resinfo[lim];
 	    if (info->opt == 'N')
-		printf("-N %2d: %-29s", lim, info->descr);
+		printf("-N %2d: %-32s ", lim, info->descr);
 	    else
-		printf("-%c: %-32s", info->opt, info->descr);
+		printf("-%c: %-35s ", info->opt, info->descr);
 	}
 	else
-	    printf("-N %2d: %-29s", lim, "");
+	    printf("-N %2d: %-32s ", lim, "");
     }
     /* display the limit */
     if (limit == RLIM_INFINITY)
@@ -511,16 +722,18 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
     rlim_t val;
     int ret = 0;
 
-    hard = OPT_ISSET(ops,'h');
-    if (OPT_ISSET(ops,'s') && !*argv)
+    hard = OPT_ISSET(ops, 'h');
+    if (OPT_ISSET(ops, 's') && !*argv)
 	return setlimits(NULL);
     /* without arguments, display limits */
     if (!*argv)
-	return showlimits(nam, hard, -1);
+	return showlimits(nam, hard, -1, OPT_ISSET(ops, 's'));
     while ((s = *argv++)) {
 	/* Search for the appropriate resource name.  When a name matches (i.e. *
 	 * starts with) the argument, the lim variable changes from -1 to the   *
 	 * number of the resource.  If another match is found, lim goes to -2.  */
+	char *err;
+
 	if (idigit(*s))
 	{
 	    lim = (int)zstrtol(s, NULL, 10);
@@ -543,61 +756,12 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
 	}
 	/* without value for limit, display the current limit */
 	if (!(s = *argv++))
-	    return showlimits(nam, hard, lim);
-	if (lim >= RLIM_NLIMITS)
-	{
-	    val = zstrtorlimt(s, &s, 10);
-	    if (*s)
-	    {
-		/* unknown limit, no idea how to scale */
-		zwarnnam(nam, "unknown scaling factor: %s", s);
-		return 1;
-	    }
-	}
-	else if (resinfo[lim]->type == ZLIMTYPE_TIME) {
-	    /* time-type resource -- may be specified as seconds, or minutes or *
-	     * hours with the `m' and `h' modifiers, and `:' may be used to add *
-	     * together more than one of these.  It's easier to understand from *
-	     * the code:                                                        */
-	    val = zstrtorlimt(s, &s, 10);
-	    if (*s) {
-		if ((*s == 'h' || *s == 'H') && !s[1])
-		    val *= 3600L;
-		else if ((*s == 'm' || *s == 'M') && !s[1])
-		    val *= 60L;
-		else if (*s == ':')
-		    val = val * 60 + zstrtorlimt(s + 1, &s, 10);
-		else {
-		    zwarnnam(nam, "unknown scaling factor: %s", s);
-		    return 1;
-		}
-	    }
-	} else if (resinfo[lim]->type == ZLIMTYPE_NUMBER ||
-		   resinfo[lim]->type == ZLIMTYPE_UNKNOWN ||
-		   resinfo[lim]->type == ZLIMTYPE_MICROSECONDS) {
-	    /* pure numeric resource -- only a straight decimal number is
-	    permitted. */
-	    char *t = s;
-	    val = zstrtorlimt(t, &s, 10);
-	    if (s == t) {
-		zwarnnam(nam, "limit must be a number");
-		return 1;
-	    }
-	} else {
-	    /* memory-type resource -- `k', `M' and `G' modifiers are *
-	     * permitted, meaning (respectively) 2^10, 2^20 and 2^30. */
-	    val = zstrtorlimt(s, &s, 10);
-	    if (!*s || ((*s == 'k' || *s == 'K') && !s[1])) {
-		if (val != RLIM_INFINITY)
-		    val *= 1024L;
-	    } else if ((*s == 'M' || *s == 'm') && !s[1])
-		val *= 1024L * 1024;
-	    else if ((*s == 'G' || *s == 'g') && !s[1])
-		val *= 1024L * 1024 * 1024;
-	    else {
-		zwarnnam(nam, "unknown scaling factor: %s", s);
-		return 1;
-	    }
+	    return showlimits(nam, hard, lim, OPT_ISSET(ops, 's'));
+
+	val = zstrtorlimt(s, lim, 0, &err);
+	if (err) {
+	    zwarnnam(nam, err);
+	    return 1;
 	}
 	if (do_limit(nam, lim, val, hard, !hard, OPT_ISSET(ops, 's')))
 	    ret++;
@@ -821,14 +985,12 @@ bin_ulimit(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		    limit = vals.rlim_max;
 		}
 	    } else {
-		limit = zstrtorlimt(*argv, &eptr, 10);
-		if (*eptr) {
-		    zwarnnam(name, "invalid number: %s", *argv);
+		char *err;
+		limit = zstrtorlimt(*argv, res, 1, &err);
+		if (err) {
+		    zwarnnam(name, "%s: %s", *argv, err);
 		    return 1;
 		}
-		/* scale appropriately */
-		if (res < RLIM_NLIMITS)
-		    limit *= resinfo[res]->unit;
 	    }
 	    if (do_limit(name, res, limit, hard, soft, 1))
 		ret++;
diff --git a/Src/Builtins/rlimits.mdd b/Src/Builtins/rlimits.mdd
index 06c9e9c7f..248a03e61 100644
--- a/Src/Builtins/rlimits.mdd
+++ b/Src/Builtins/rlimits.mdd
@@ -3,6 +3,12 @@ link=either
 load=yes
 
 autofeatures="b:limit b:ulimit b:unlimit"
-autofeatures_emu="b:ulimit"
+
+# limit is the csh builtin, while ulimit is the ksh/posix one.
+# Autoloading ulimit in csh emulation should be relatively
+# harmless as "ulimit" contrary to "limit" is not otherwise
+# a common English word. So we're only accomodating sh/ksh
+# emulations.
+autofeatures_posix="b:ulimit"
 
 objects="rlimits.o"
diff --git a/Src/mkbltnmlst.sh b/Src/mkbltnmlst.sh
index c4611d8b3..7ebc2a751 100644
--- a/Src/mkbltnmlst.sh
+++ b/Src/mkbltnmlst.sh
@@ -37,10 +37,10 @@ for x_mod in $x_mods; do
         echo "/* non-linked-in known module \`$x_mod' */"
 	linked=no
     esac
-    unset moddeps autofeatures autofeatures_emu
+    unset moddeps autofeatures autofeatures_posix
     . $srcdir/../$modfile
     if test "x$autofeatures" != x; then
-        if test "x$autofeatures_emu" != x; then
+        if test "x$autofeatures_posix" != x; then
             echo "  {"
 	    echo "    char *zsh_features[] = { "
 	    for feature in $autofeatures; do
@@ -48,14 +48,14 @@ for x_mod in $x_mods; do
 	    done
 	    echo "      NULL"
 	    echo "    }; "
-	    echo "    char *emu_features[] = { "
-	    for feature in $autofeatures_emu; do
+	    echo "    char *posix_features[] = { "
+	    for feature in $autofeatures_posix; do
 		echo "      \"$feature\","
 	    done
 	    echo "      NULL"
 	    echo "    }; "
 	    echo "    autofeatures(\"zsh\", \"$x_mod\","
-	    echo "       EMULATION(EMULATE_ZSH) ? zsh_features : emu_features,"
+	    echo "       EMULATION(EMULATE_KSH|EMULATE_SH) ? posix_features : zsh_features,"
 	    echo "       0, 1);"
 	    echo "  }"
         else
diff --git a/Test/B12limit.ztst b/Test/B12limit.ztst
index 48d33e6e3..3665279fa 100644
--- a/Test/B12limit.ztst
+++ b/Test/B12limit.ztst
@@ -26,3 +26,132 @@ F:report this to zsh-workers mailing list.
   }
 0:check if limit option letters are unique
 
+  if sh -c 'ulimit 2048' > /dev/null 2>&1; then
+    (
+      set -o braceccl -o pipefail
+      list=(1b 1{kmgtpe}{,b,ib})
+      for cmd in "limit filesize" ulimit; do
+	for l in $list $list:u; do
+	  $=cmd $l &&
+	    limit filesize &&
+	    ulimit || exit
+	done
+      done | sed 'N;s/\n/ /;s/  */ /g'
+    )
+  else
+    ZTST_skip='Cannot set the filesize limit on this system'
+  fi
+0:filesize suffixes with limit and ulimit
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+
+  if sh -c 'ulimit -t 3600' > /dev/null 2>&1; then
+  (
+    set -o pipefail
+    list=(1h 30m 20s 30 1:23:45.123456 2:23 56.4)
+    for cmd in "limit cputime" "ulimit -t"; do
+      for l in $list ${(MU)list:#*[a-z]*}; do
+        $=cmd $l &&
+	  limit cputime &&
+	  ulimit -t || exit
+      done
+    done | sed 'N;s/\n/ /;s/  */ /g'
+  )
+  else
+    ZTST_skip='Cannot set the cputime limit on this system'
+  fi
+0:time limit formats
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 0:00:30 30
+>cputime 1:23:45 5025
+>cputime 0:02:23 143
+>cputime 0:00:56 56
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 0:00:30 30
+>cputime 1:23:45 5025
+>cputime 0:02:23 143
+>cputime 0:00:56 56
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
-- 
2.25.1



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-23 21:49 [PATCHv1] [long] improvements to limit/ulimit API and doc Stephane Chazelas
@ 2020-11-25  0:35 ` Daniel Shahaf
  2020-11-25  6:44   ` Stephane Chazelas
  2020-11-26  6:57   ` [PATCHv2 1/2] [long] improvements to limit/ulimit API and doc ((un)limit in csh emulation) Stephane Chazelas
  2020-11-25 23:43 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Oliver Kiddle
  2020-11-26 11:19 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Jun T
  2 siblings, 2 replies; 30+ messages in thread
From: Daniel Shahaf @ 2020-11-25  0:35 UTC (permalink / raw)
  To: Zsh hackers list

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@chazelas.org>
> 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.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-25  0:35 ` Daniel Shahaf
@ 2020-11-25  6:44   ` Stephane Chazelas
  2020-11-27 17:16     ` Daniel Shahaf
  2020-11-26  6:57   ` [PATCHv2 1/2] [long] improvements to limit/ulimit API and doc ((un)limit in csh emulation) Stephane Chazelas
  1 sibling, 1 reply; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-25  6:44 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

2020-11-25 00:35:12 +0000, Daniel Shahaf:
[...]
> > 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.
[...]

Hi Daniel,

thanks for having looking into it.

They're not independent, except maybe for the "limit"/"unlimit"
now back in csh which is a 5 code line change.

Chunks of the code and doc have been rewritten to address
those. Separating out the changes would mean rewriting the code
several times which would be more effort for me and reviewers,
mostly wasted if we're only keeping the last iteration.

But maybe before looking in detail at the code, we can discuss
whether the change in API makes sense.


> > * 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.
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-23 21:49 [PATCHv1] [long] improvements to limit/ulimit API and doc Stephane Chazelas
  2020-11-25  0:35 ` Daniel Shahaf
@ 2020-11-25 23:43 ` Oliver Kiddle
  2020-11-26 20:14   ` [PATCH] ulimit option completions using ulimit -a output Stephane Chazelas
  2020-11-26 20:58   ` [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
  2020-11-26 11:19 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Jun T
  2 siblings, 2 replies; 30+ messages in thread
From: Oliver Kiddle @ 2020-11-25 23:43 UTC (permalink / raw)
  To: Zsh hackers list, Stephane Chazelas

Stephane Chazelas wrote:
> - I've only tested it on Ubuntu, FreeBSD and Cygwin. I suspect
>   my test cases will fail on 32bit systems. They're skipped on

I tried building and running tests on Solaris 11 and a 32bit build on
Solaris 10. I also checked OpenBSD, NetBSD and Dragonfly. There were
quite a few other issues but nothing introduced by your patch and
your tests passed. Let me know if there's anything additionally that
it'd be useful for me to test manually.

I've got a few comments, mostly on the documentation.

> -If no var(resource) is given, print all limits.
> +If tt(-s) is given without other arguments, the resource limits of the
> +current shell is set to the previously set resource limits of the

s/is/are/

> +sitem(tt(rt_priority))(Maximum realtime priority.)
> +sitem(tt(rt_time))(Maximum realtime CPU time slice.)

From the tcsh manual, these appear to be called maxrtprio and maxrttime
there. But we already had these names, right? Similarly tcsh has
posixlocks.

> -var(limit) is a number, with an optional scaling factor, as follows:
> +If var(limit) is a decimal integer number without suffix, then for
> +historical reason and compatibility with csh where that command comes from,

s/reason/reasons/
Or perhaps just drop the first three words of that line.

> +the unit will depend on the type of limit: microsecond for tt(rt_time),
> +second for all other time limits, kibibytes (1024 bytes) for all limits that

"microseconds" and "seconds" - units are mostly always plural.

> +The same suffixes are recognised as for tt(limit), but bear in mind that
> +default units when no suffix is specified varry between the two.

s/varry/vary/

> +performed with kibibytes, mebibytes, or blocks (of 512 bytes; catch: not
> +em(pebibytes)) instead. (On some systems additional specifiers are available

Not sure about the use of "catch:" there. I'd likely apply emphasis to
not instead

> +The `limit` builtin accepts more units for resource limits, now shared
> +with the `ulimit` builtin allowing to specify arbitrary limit values.

"allowing to" isn't correct unless you add an object for the verb "allow"
such as "you". It might be better to reword it in the passive tense ("to
be specified").

> +`limit -s somelimit` now reports the shell process' own value of the limit.
> +The `limit` and `unlimit` builtins are now available again in csh emulation.
> +More generally, the resouce limit handling interfaces and documentation have

"resource".
"More generally" doesn't really add anything useful to the sentence.

> +++ b/Src/Builtins/rlimits.c
> @@ -55,7 +55,8 @@ typedef struct resinfo_T {
>   * 1. Add zsh_LIMIT_PRESENT(RLIMIT_XXX) in configure.ac.
>   * 2. Add an entry for RLIMIT_XXX to known_resources[].
>   *    Make sure the option letter (resinto_T.opt) is unique.
> - * 3. Build zsh and run the test B12rlimit.ztst.
> + * 3. Add entry in documentation for the limit and ulimit builtins
> + * 4. Build zsh and run the test B12rlimit.ztst.

5. Update Completion/Zsh/Command/_ulimit

Units were often fairly well documented there but are missing in a
couple of cases so you may be able to add a couple.

>  0:check if limit option letters are unique
> 
> +  if sh -c 'ulimit 2048' > /dev/null 2>&1; then

This doesn't work on Solaris or any of the BSDs except FreeBSD - you
have to specify -f to limit file sizes. So it would be skipped despite
the fact that the test could otherwise work.
Isn't -f fairly standard?

From my testing of the changes, it definitely looks like a nice improvement.

Oliver



^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCHv2 1/2] [long] improvements to limit/ulimit API and doc ((un)limit in csh emulation)
  2020-11-25  0:35 ` Daniel Shahaf
  2020-11-25  6:44   ` Stephane Chazelas
@ 2020-11-26  6:57   ` Stephane Chazelas
  1 sibling, 0 replies; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-26  6:57 UTC (permalink / raw)
  To: zsh-workers

2020-11-25 00:35:12 +0000, Daniel Shahaf:
[...]
> 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.
[...]

Here is for the csh builtins, I'll send a second one for the
rest addressing Oliver's comments (doc, testcase, _ulimit)
hopefully tonight. I'll add a test case to verify we get errors
upon unexpected suffixes as well.

From: Stephane Chazelas <stephane@chazelas.org>
Date: Thu, 26 Nov 2020 06:34:44 +0000
Subject: [PATCH] Re-enable `limit` and `unlimit` in csh emulation.

Those two builtins do come from csh (4BSD, 1980 along with resource
limits) in the first place, they were disabled when emulating *other*
shells, that should only have been for POSIX/ksh which have ulimit and
not limit.

  -> 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.
---
 Doc/Zsh/builtins.yo      |  4 ++--
 NEWS                     |  2 ++
 Src/Builtins/rlimits.mdd |  8 +++++++-
 Src/mkbltnmlst.sh        | 10 +++++-----
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index ebb29f632..2f1ccd8a5 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1180,7 +1180,7 @@ sitem([var(mm)tt(:)]var(ss))(minutes and seconds)
 endsitem()
 
 The tt(limit) command is not made available by default when the
-shell starts in a mode emulating another shell.  It can be made available
+shell starts in sh or ksh emulation mode.  It can be made available
 with the command `tt(zmodload -F zsh/rlimits b:limit)'.
 )
 findex(local)
@@ -2343,7 +2343,7 @@ The resources of the shell process are only changed if the tt(-s)
 flag is given.
 
 The tt(unlimit) command is not made available by default when the
-shell starts in a mode emulating another shell.  It can be made available
+shell starts in sh or ksh emulation mode.  It can be made available
 with the command `tt(zmodload -F zsh/rlimits b:unlimit)'.
 )
 findex(unset)
diff --git a/NEWS b/NEWS
index a8e7df80e..d05e8b64f 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,8 @@ The zsh/system module's `zsystem flock` command learnt an -i option to
 set the wait interval used with -t. Additionally, -t now supports
 fractional seconds.
 
+The `limit` and `unlimit` builtins are now available again in csh emulation.
+
 Changes from 5.7.1-test-3 to 5.8
 --------------------------------
 
diff --git a/Src/Builtins/rlimits.mdd b/Src/Builtins/rlimits.mdd
index 06c9e9c7f..248a03e61 100644
--- a/Src/Builtins/rlimits.mdd
+++ b/Src/Builtins/rlimits.mdd
@@ -3,6 +3,12 @@ link=either
 load=yes
 
 autofeatures="b:limit b:ulimit b:unlimit"
-autofeatures_emu="b:ulimit"
+
+# limit is the csh builtin, while ulimit is the ksh/posix one.
+# Autoloading ulimit in csh emulation should be relatively
+# harmless as "ulimit" contrary to "limit" is not otherwise
+# a common English word. So we're only accomodating sh/ksh
+# emulations.
+autofeatures_posix="b:ulimit"
 
 objects="rlimits.o"
diff --git a/Src/mkbltnmlst.sh b/Src/mkbltnmlst.sh
index c4611d8b3..7ebc2a751 100644
--- a/Src/mkbltnmlst.sh
+++ b/Src/mkbltnmlst.sh
@@ -37,10 +37,10 @@ for x_mod in $x_mods; do
         echo "/* non-linked-in known module \`$x_mod' */"
 	linked=no
     esac
-    unset moddeps autofeatures autofeatures_emu
+    unset moddeps autofeatures autofeatures_posix
     . $srcdir/../$modfile
     if test "x$autofeatures" != x; then
-        if test "x$autofeatures_emu" != x; then
+        if test "x$autofeatures_posix" != x; then
             echo "  {"
 	    echo "    char *zsh_features[] = { "
 	    for feature in $autofeatures; do
@@ -48,14 +48,14 @@ for x_mod in $x_mods; do
 	    done
 	    echo "      NULL"
 	    echo "    }; "
-	    echo "    char *emu_features[] = { "
-	    for feature in $autofeatures_emu; do
+	    echo "    char *posix_features[] = { "
+	    for feature in $autofeatures_posix; do
 		echo "      \"$feature\","
 	    done
 	    echo "      NULL"
 	    echo "    }; "
 	    echo "    autofeatures(\"zsh\", \"$x_mod\","
-	    echo "       EMULATION(EMULATE_ZSH) ? zsh_features : emu_features,"
+	    echo "       EMULATION(EMULATE_KSH|EMULATE_SH) ? posix_features : zsh_features,"
 	    echo "       0, 1);"
 	    echo "  }"
         else
-- 
2.25.1




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-23 21:49 [PATCHv1] [long] improvements to limit/ulimit API and doc Stephane Chazelas
  2020-11-25  0:35 ` Daniel Shahaf
  2020-11-25 23:43 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Oliver Kiddle
@ 2020-11-26 11:19 ` Jun T
  2020-11-26 13:55   ` Stephane Chazelas
  2 siblings, 1 reply; 30+ messages in thread
From: Jun T @ 2020-11-26 11:19 UTC (permalink / raw)
  To: zsh-workers

This is not the problem of the patch, but I noticed limit/ulimit of zsh
has a small problem due to the wrap-around of unsigned integers.
For example, on Linux, either with or without Stephane's patch:

zsh% ulimit 36028797018963967 && ulimit
36028797018963967                          # = 2**55 - 1
zsh% ulimit 36028797018963968 && ulimit
0

This is because 36028797018963968 blocks = 2**64, which wrap around to zero.
With the patch we can more easily see:

zsh% limit filesize 8EiB && limit filesize
filesize        8EiB
zsh% limit filesize 16EiB && limit fillesize
filesize        0B           # 16EiB = 2**64 = 0

It seems bash is doing some check for the wrap around:

zsh% bash --posix            # block=512B in POSIX mode
bash$ ulimit 36028797018963967 && ulimit
36028797018963967
bash$ ulimit 36028797018963968
bash: ulimit: 36028797018963968: limit out of range



In B12limit.ztst

> +  if sh -c 'ulimit 2048' > /dev/null 2>&1; then

Can we assume that there is no hard limit set by the System?
Later in the test we will test up to 1EiB, so maybe it would be safer
to use 'ulimit -f unlimited'?
(All the systems I know do not set any hard limit on filesize
so maybe we need not bother with this).



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-26 11:19 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Jun T
@ 2020-11-26 13:55   ` Stephane Chazelas
  2020-11-26 15:22     ` Jun. T
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-26 13:55 UTC (permalink / raw)
  To: Jun T; +Cc: zsh-workers

2020-11-26 20:19:37 +0900, Jun T:
> This is not the problem of the patch, but I noticed limit/ulimit of zsh
> has a small problem due to the wrap-around of unsigned integers.
> For example, on Linux, either with or without Stephane's patch:
> 
> zsh% ulimit 36028797018963967 && ulimit
> 36028797018963967                          # = 2**55 - 1
> zsh% ulimit 36028797018963968 && ulimit
> 0
> 
> This is because 36028797018963968 blocks = 2**64, which wrap around to zero.
> With the patch we can more easily see:
> 
> zsh% limit filesize 8EiB && limit filesize
> filesize        8EiB
> zsh% limit filesize 16EiB && limit fillesize
> filesize        0B           # 16EiB = 2**64 = 0
> 
> It seems bash is doing some check for the wrap around:
> 
> zsh% bash --posix            # block=512B in POSIX mode
> bash$ ulimit 36028797018963967 && ulimit
> 36028797018963967
> bash$ ulimit 36028797018963968
> bash: ulimit: 36028797018963968: limit out of range

But:

$ bash --posix -c 'ulimit 18446744073709551617;  ulimit'
1

There's also the question of the actual value of RLIM_INFINITY,
RLIM_SAVED_MAX, RLIM_SAVED_CUR and whether we can assume
RLIM_INFINITY is the maximum possible value.

POSIX doesn't seem to give us much guarantee there
(https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/basedefs/sys_resource.h.html)


> In B12limit.ztst
> 
> > +  if sh -c 'ulimit 2048' > /dev/null 2>&1; then
> 
> Can we assume that there is no hard limit set by the System?
> Later in the test we will test up to 1EiB, so maybe it would be safer
> to use 'ulimit -f unlimited'?
> (All the systems I know do not set any hard limit on filesize
> so maybe we need not bother with this).

Yes, "make test" is meant for packagers. I'd say it's up to them
to update their environment so the tests can run.

Now, I'd expect those 1EiB tests would fail on some system
whether or not "ulimit -f unlimited" works or not. I haven't
found any yet. But if there are any, it's probably better we be
told about it (and a failing test is a good way to achieve
that).

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-26 13:55   ` Stephane Chazelas
@ 2020-11-26 15:22     ` Jun. T
  2020-11-26 17:23       ` Stephane Chazelas
  0 siblings, 1 reply; 30+ messages in thread
From: Jun. T @ 2020-11-26 15:22 UTC (permalink / raw)
  To: zsh-workers


> 2020/11/26 22:55, Stephane Chazelas <stephane@chazelas.org> wrote:
> 
> But:
> 
> $ bash --posix -c 'ulimit 18446744073709551617;  ulimit'
> 1

I think this is a bug of bash.
It seems bash checks the wrap around only for the calculation of
bytes = 512*blocks, and does not check if the number of blocks
is already wrapped around.

> There's also the question of the actual value of RLIM_INFINITY,
> RLIM_SAVED_MAX, RLIM_SAVED_CUR and whether we can assume
> RLIM_INFINITY is the maximum possible value.

I guess zsh need not care about these things.

# I must say I don't know what it means if RLIM_SAVED_XXX is not
# equal to RLIMI_INFINITY.

Zsh only need to check whether the new limit user wants to set is
within the range of rlim_t.

If it is not, then zsh should warn about it instead of passing the
wrap-around value to setrlimit(2).

If it is, then just pass it to setrlimit(2) (need not check whether
it is smaller than RLIM_INFINITY or not).
If setrlimit() returns error, then report it to the user.

Jun




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-26 15:22     ` Jun. T
@ 2020-11-26 17:23       ` Stephane Chazelas
  2020-11-27 18:24         ` Jun. T
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-26 17:23 UTC (permalink / raw)
  To: Jun. T; +Cc: zsh-workers

2020-11-27 00:22:05 +0900, Jun. T:
> 
> > 2020/11/26 22:55, Stephane Chazelas <stephane@chazelas.org> wrote:
> > 
> > But:
> > 
> > $ bash --posix -c 'ulimit 18446744073709551617;  ulimit'
> > 1
> 
> I think this is a bug of bash.
> It seems bash checks the wrap around only for the calculation of
> bytes = 512*blocks, and does not check if the number of blocks
> is already wrapped around.

Yes, looks like it does:

if (limit * factor / factor != limit) error(out of range)

(where limit is converted from string without overflow check).

> 
> > There's also the question of the actual value of RLIM_INFINITY,
> > RLIM_SAVED_MAX, RLIM_SAVED_CUR and whether we can assume
> > RLIM_INFINITY is the maximum possible value.
> 
> I guess zsh need not care about these things.
> 
> # I must say I don't know what it means if RLIM_SAVED_XXX is not
> # equal to RLIM_INFINITY.

I can't say I know anything about those RLIM_SAVED_XXX. I
came across them for the first time today.

> Zsh only need to check whether the new limit user wants to set is
> within the range of rlim_t.

Yes, but how do you determine that range? Should we not also
reject 18446744073709551615 as out-of-range on systems where
it's RLIM_INFINITY since it's not preventing file sizes to get
past 18446744073709551615 for instance.

$ bash --posix -c 'ulimit -t 18446744073709551615 && ulimit -t'
unlimited

yash rejects it:

$ yash -c 'ulimit -t 18446744073709551614 && ulimit -t'
18446744073709551614
$ yash -c 'ulimit -t 18446744073709551615 && ulimit -t'
ulimit: Numerical result out of range
$ yash -c 'ulimit -t 18446744073709551616 && ulimit -t'
ulimit: `18446744073709551616' is not a valid integer

In any case, while I agree they are valid (though minor)
concerns, I won't address them in this round.  I can add a
"BUGS" entry though.

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH] ulimit option completions using ulimit -a output
  2020-11-25 23:43 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Oliver Kiddle
@ 2020-11-26 20:14   ` Stephane Chazelas
  2020-11-27  7:13     ` Stephane Chazelas
  2021-04-13 14:35     ` Daniel Shahaf
  2020-11-26 20:58   ` [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
  1 sibling, 2 replies; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-26 20:14 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Zsh hackers list

2020-11-26 00:43:33 +0100, Oliver Kiddle:
[...]
> >   * 1. Add zsh_LIMIT_PRESENT(RLIMIT_XXX) in configure.ac.
> >   * 2. Add an entry for RLIMIT_XXX to known_resources[].
> >   *    Make sure the option letter (resinto_T.opt) is unique.
> > - * 3. Build zsh and run the test B12rlimit.ztst.
> > + * 3. Add entry in documentation for the limit and ulimit builtins
> > + * 4. Build zsh and run the test B12rlimit.ztst.
> 
> 5. Update Completion/Zsh/Command/_ulimit
> 
> Units were often fairly well documented there but are missing in a
> couple of cases so you may be able to add a couple.
[...]

The problem is that the list of ulimit options is system
dependent, so I propose the improvement below which uses the
output of ulimit -a to build the list of completions.

From: Stephane Chazelas <stephane@chazelas.org>
Date: Thu, 26 Nov 2020 19:22:03 +0000
Subject: [PATCH] ulimit option completions using ulimit -a output

The list of options supported by ulimit is system dependent. Rather than
hardcoding the full list of options supported on any system in the
completer, we get the list of options from the output of "ulimit -a".

That means less descriptive descriptions but more relevant completions
to the user (and not having to update the completer every time a new
limit is added).
---
 Completion/Zsh/Command/_ulimit | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/Completion/Zsh/Command/_ulimit b/Completion/Zsh/Command/_ulimit
index 0526821dd..06bcfc34c 100644
--- a/Completion/Zsh/Command/_ulimit
+++ b/Completion/Zsh/Command/_ulimit
@@ -2,18 +2,16 @@
 
 [[ $PREFIX = u* ]] && compadd unlimited && return 0
 
+local -a opts
+setopt localoptions extendedglob
+
+opts=(
+  ${${${(fo)"$(ulimit -a)"}:#-N*}%%  *}
+)
 _arguments -s \
   '-H[set hard limits]' \
   '-S[set soft and hard limits (with -H)]' \
-  '(-H -S -c -d -f -l -m -n -s -t *)-a[list all current resource limits]' \
-  '-c[core dump size limit]:max core dump size (512-byte blocks)' \
-  '-d[maximum size of data segment]:maximum size of data segment (K-bytes)' \
-  '-f[size of largest file allowed]:size of largest file allowed (512-byte blocks)' \
-  '-l[maximum size of locked in memory]:maximum size of locked in memory (K-bytes)' \
-  '-m[maximum size of physical memory]:maximum size of physical memory (K-bytes)' \
-  '-n[maximum no. of open file descriptors]:maximum no. of open file descriptors' \
-  '-s[stack size limit]:stack size limit (K-bytes)' \
-  '-t[maximum cpu time per process]:maximum cpu time per process (seconds)' \
-  '-u[processes available to the user]:processes' \
-  '-v[maximum size of virtual memory]:maximum size of virtual memory (K-bytes)' \
-  '*:size of largest file allowed'
+  '-N+[specify limit by number]:limit number' \
+  "(-N ${(M@j: :)opts#-?} *)-a[list all current resource limits]" \
+  ${opts/(#b)(-?): (*)/$match[1][$match[2]]:$match[2]} \
+  '*:file size (blocks)'
-- 
2.25.1




^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2020-11-25 23:43 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Oliver Kiddle
  2020-11-26 20:14   ` [PATCH] ulimit option completions using ulimit -a output Stephane Chazelas
@ 2020-11-26 20:58   ` Stephane Chazelas
  2020-11-27 16:39     ` Daniel Shahaf
  1 sibling, 1 reply; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-26 20:58 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Zsh hackers list

2020-11-26 00:43:33 +0100, Oliver Kiddle:
[...]
> I've got a few comments, mostly on the documentation.
[...]

Thanks for all the corrections. As you might have guessed,
English is not my first language, but a few of them I should
have been able to avoid, sorry about that.

Below is the second iteration, integrating Oliver's comments.
That's the second part, exluding the csh mode change (1/2) which
was separated out (still needs to applied before this one).

I've also added a test case to check that invalid input is
correctly rejected.

The change for _ulimit was addressed in a separate patch sent
earlier.

(there are still a few outstanding questions in my original
message that we may want to discuss/address).


From: Stephane Chazelas <stephane@chazelas.org>
Date: Thu, 26 Nov 2020 20:38:57 +0000
Subject: [PATCH] improve limit/ulimit API and documentation

A few issues addressed:

* 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` 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    | 125 +++++++++----
 Doc/Zsh/expn.yo        |  18 +-
 Doc/Zsh/mod_zpty.yo    |   4 +-
 Doc/Zsh/params.yo      |  10 +-
 Etc/BUGS               |   4 +
 NEWS                   |   5 +
 Src/Builtins/rlimits.c | 397 +++++++++++++++++++++++++++++------------
 Test/B12limit.ztst     | 157 ++++++++++++++++
 8 files changed, 553 insertions(+), 167 deletions(-)

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 2f1ccd8a5..5a2c51b0b 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1116,15 +1116,18 @@ cindex(resource limits)
 cindex(limits, resource)
 item(tt(limit) [ tt(-hs) ] [ var(resource) [ var(limit) ] ] ...)(
 Set or display resource limits.  Unless the tt(-s) flag is given,
-the limit applies only the children of the shell.  If tt(-s) is
-given without other arguments, the resource limits of the current
-shell is set to the previously set resource limits of the children.
+the limit applies only to child processes and executed commands, not the
+current shell process itself.
 
-If var(limit) is not specified, print the current limit placed
-on var(resource), otherwise
-set the limit to the specified value.  If the tt(-h) flag
-is given, use hard limits instead of soft limits.
-If no var(resource) is given, print all limits.
+If tt(-s) is given without other arguments, the resource limits of the
+current shell are set to the previously set resource limits of the
+children.
+
+If var(limit) is not specified, print the current limit placed on
+var(resource) (of the shell with tt(-s), of children without), otherwise
+set the limit to the specified value.  If the tt(-h) flag is given, use
+hard limits instead of soft limits. If no var(resource) is given, print
+all limits.
 
 When looping over multiple resources, the shell will abort immediately if
 it detects a badly formed argument.  However, if it fails to set a limit
@@ -1143,18 +1146,23 @@ sitem(tt(datasize))(Maximum data size (including stack) for each process.)
 sitem(tt(descriptors))(Maximum value for a file descriptor.)
 sitem(tt(filesize))(Largest single file allowed.)
 sitem(tt(kqueues))(Maximum number of kqueues allocated.)
+sitem(tt(maxfilelocks))(Maximum number of file locks.)
 sitem(tt(maxproc))(Maximum number of processes.)
-sitem(tt(maxpthreads))(Maximum number of threads per process.)
+sitem(tt(maxpthreads))(Maximum number of threads per uid (NetBSD/OpenBSD) or per process.)
 sitem(tt(memorylocked))(Maximum amount of memory locked in RAM.)
 sitem(tt(memoryuse))(Maximum resident set size.)
 sitem(tt(msgqueue))(Maximum number of bytes in POSIX message queues.)
+sitem(tt(nice))(Maximum nice priority.)
 sitem(tt(posixlocks))(Maximum number of POSIX locks per user.)
 sitem(tt(pseudoterminals))(Maximum number of pseudo-terminals.)
 sitem(tt(resident))(Maximum resident set size.)
+sitem(tt(rt_priority))(Maximum realtime priority.)
+sitem(tt(rt_time))(Maximum realtime CPU time slice.)
 sitem(tt(sigpending))(Maximum number of pending signals.)
 sitem(tt(sockbufsize))(Maximum size of all socket buffers.)
 sitem(tt(stacksize))(Maximum stack size for each process.)
 sitem(tt(swapsize))(Maximum amount of swap used.)
+sitem(tt(umtxp))(Maximum number of umtx shared locks.)
 sitem(tt(vmemorysize))(Maximum amount of virtual memory.)
 endsitem()
 
@@ -1169,14 +1177,42 @@ the limit anyway, and will report an error if this fails.  As the shell
 does not store such resources internally, an attempt to set the limit will
 fail unless the tt(-s) option is present.
 
-var(limit) is a number, with an optional scaling factor, as follows:
+If var(limit) is a decimal integer number without suffix, then for
+historical reasons and compatibility with csh where that command comes from,
+the unit will depend on the type of limit: microseconds for tt(rt_time),
+seconds for all other time limits, kibibytes (1024 bytes) for all limits that
+express a number of bytes (except tt(msgqueue)).
+
+Instead, to avoid confusion, a suffix (case insensitive) may be appended to
+the (integer only) decimal number to specify the unit:
 
 startsitem()
-sitem(var(n)tt(h))(hours)
-sitem(var(n)tt(k))(kilobytes (default))
-sitem(var(n)tt(m))(megabytes or minutes)
-sitem(var(n)tt(g))(gigabytes)
-sitem([var(mm)tt(:)]var(ss))(minutes and seconds)
+sitem(time limits)(
+startsitem()
+sitem(tt(h))(hours)
+sitem(tt(m))(minutes)
+sitem(tt(s))(seconds)
+sitem(tt(ms))(milliseconds)
+sitem(tt(us))(microseconds)
+sitem(tt([[var(h):]var(m):]var(s)[.var(usec)]))(all combined)
+endsitem()
+
+For instance a 2 millisecond limit of tt(rt_time), can be expressed as
+tt(2ms), tt(2000us), tt(0.002), tt(0:0:0.002) or tt(2000) without unit.)
+sitem(memory limits)(
+startsitem()
+sitem(tt(k, kib))(kibibytes (1024 bytes))
+sitem(tt(m, mib))(mibibytes)
+sitem(...)(and so on with tt(g), tt(t), tt(p), tt(e) for gibibytes,
+tebibytes, pebibytes and exbibytes respectively)
+sitem(tt(kb))(kilobytes (1,000 bytes))
+sitem(tt(mb))(megabytes (1,000,000 bytes))
+sitem(...)(and so on with tt(g), tt(t), tt(p), tt(e) for gigabytes,
+terabytes, petabytes and exabytes respectively.)
+sitem(tt(b))(em(byte) to set the limit with arbitrary precisions)
+endsitem())
+sitem(other limits)(other limits are assumed to be numerical and ony
+a decimal integer number is accepted.)
 endsitem()
 
 The tt(limit) command is not made available by default when the
@@ -2250,8 +2286,8 @@ together with the tt(-H) flag set both hard and soft limits.
 If no options are used, the file size limit (tt(-f)) is assumed.
 
 If var(limit) is omitted the current value of the specified resources are
-printed.  When more than one resource value is printed, the limit name and
-unit is printed before each value.
+printed (rounded down to the corresponding unit).  When more than one resource
+value is printed, the limit name and unit is printed before each value.
 
 When looping over multiple resources, the shell will abort immediately if
 it detects a badly formed argument.  However, if it fails to set a limit
@@ -2260,30 +2296,45 @@ for some other reason it will continue trying to set the remaining limits.
 Not all the following resources are supported on all systems.  Running
 tt(ulimit -a) will show which are supported.
 
+Whilst tt(limit) is the BSD/csh-style command, tt(ulimit) is the SysV/Korn
+shell equivalent (added later to zsh for compatibility with ksh). While
+tt(limit) in zsh sets the limits for child processes by default, tt(ulimit)
+sets them for both the shell and child processes, and reports the ones
+set for child processes.
+
+The same suffixes are recognised as for tt(limit), but bear in mind that
+default units when no suffix is specified vary between the two.
+
+Below, the corresponding tt(limit) keyword for each tt(ulimit) option is
+shown in parenthesis for reference:
+
 startsitem()
 sitem(tt(-a))(Lists all of the current resource limits.)
-sitem(tt(-b))(Socket buffer size in bytes LPAR()N.B. not kilobytes+RPAR())
-sitem(tt(-c))(512-byte blocks on the size of core dumps.)
-sitem(tt(-d))(Kilobytes on the size of the data segment.)
-sitem(tt(-f))(512-byte blocks on the size of files written.)
-sitem(tt(-i))(The number of pending signals.)
-sitem(tt(-k))(The number of kqueues allocated.)
-sitem(tt(-l))(Kilobytes on the size of locked-in memory.)
-sitem(tt(-m))(Kilobytes on the size of physical memory.)
-sitem(tt(-n))(open file descriptors.)
-sitem(tt(-p))(The number of pseudo-terminals.)
-sitem(tt(-q))(Bytes in POSIX message queues.)
+sitem(tt(-b))(Socket buffer size in bytes LPAR()N.B. not kibibytes+RPAR() (tt(sockbufsize)).)
+sitem(tt(-c))(512-byte blocks on the size of core dumps (tt(coredumpsize)).)
+sitem(tt(-d))(Kibibytes on the size of the data segment (tt(datasize)).)
+sitem(tt(-e))(nice priority (tt(nice)).)
+sitem(tt(-f))(512-byte blocks on the size of files written (tt(filesize)).)
+sitem(tt(-i))(The number of pending signals (tt(sigpending)).)
+sitem(tt(-k))(The number of kqueues allocated (tt(kqueues)).)
+sitem(tt(-l))(Kibibytes on the size of locked-in memory (tt(memorylocked)).)
+sitem(tt(-m))(Kibibytes on the size of physical memory (tt(resident)).)
+sitem(tt(-n))(open file descriptors (tt(descriptors)).)
+sitem(tt(-o))(umtx shared locks (tt(umtxp)).)
+sitem(tt(-p))(The number of pseudo-terminals (tt(pseudoterminals)).)
+sitem(tt(-q))(Bytes in POSIX message queues (tt(msgqueue)).)
 sitem(tt(-r))(Maximum real time priority.  On some systems where this
 is not available, such as NetBSD, this has the same effect as tt(-T)
-for compatibility with tt(sh).)
-sitem(tt(-s))(Kilobytes on the size of the stack.)
-sitem(tt(-T))(The number of simultaneous threads available to the user.)
-sitem(tt(-t))(CPU seconds to be used.)
-sitem(tt(-u))(The number of processes available to the user.)
-sitem(tt(-v))(Kilobytes on the size of virtual memory.  On some systems this
-refers to the limit called `address space'.)
-sitem(tt(-w))(Kilobytes on the size of swapped out memory.)
-sitem(tt(-x))(The number of locks on files.)
+for compatibility with tt(sh) (tt(rt_priority) / tt(maxpthreads)).)
+sitem(tt(-R))(realtime CPU time slice (tt(rt_time)).)
+sitem(tt(-s))(Kibibytes on the size of the stack (tt(stacksize)).)
+sitem(tt(-T))(The number of simultaneous threads available to the user (tt(maxpthreads)).)
+sitem(tt(-t))(CPU seconds to be used (tt(cputime)).)
+sitem(tt(-u))(The number of processes available to the user (tt(maxproc)).)
+sitem(tt(-v))(Kibibytes on the size of virtual memory.  On some systems this
+refers to the limit called em(address space) (tt(vmemorysize) / tt(addressspace)).)
+sitem(tt(-w))(Kibibytes on the size of swapped out memory (tt(swapsize)).)
+sitem(tt(-x))(The number of locks on files (tt(maxfilelocks)).)
 endsitem()
 
 A resource may also be specified by integer in the form `tt(-N)
diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index b3396721f..259c00a6c 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -2837,15 +2837,15 @@ exactly var(n) bytes in length.
 
 If this flag is directly followed by a em(size specifier) `tt(k)' (`tt(K)'),
 `tt(m)' (`tt(M)'), or `tt(p)' (`tt(P)') (e.g. `tt(Lk-50)') the check is
-performed with kilobytes, megabytes, or blocks (of 512 bytes) instead.
-(On some systems additional specifiers are available for gigabytes,
-`tt(g)' or `tt(G)', and terabytes, `tt(t)' or `tt(T)'.) If a size specifier
-is used a file is regarded as "exactly" the size if the file size rounded up
-to the next unit is equal to the test size.  Hence `tt(*LPAR()Lm1+RPAR())'
-matches files from 1 byte up to 1 Megabyte inclusive.  Note also that
-the set of files "less than" the test size only includes files that would
-not match the equality test; hence `tt(*LPAR()Lm-1+RPAR())' only matches
-files of zero size.
+performed with kibibytes, mebibytes, or blocks (of 512 bytes; note: em(not)
+pebibytes) instead. (On some systems additional specifiers are available
+for gibibytes, `tt(g)' or `tt(G)', and tebibytes, `tt(t)' or `tt(T)'.) If a
+size specifier is used, a file is regarded as "exactly" the size if the file
+size rounded up to the next unit is equal to the test size.  Hence
+`tt(*LPAR()Lm1+RPAR())' matches files from 1 byte up to 1 Mebibyte
+inclusive.  Note also that the set of files "less than" the test size only
+includes files that would not match the equality test; hence
+`tt(*LPAR()Lm-1+RPAR())' only matches files of zero size.
 )
 item(tt(^))(
 negates all qualifiers following it
diff --git a/Doc/Zsh/mod_zpty.yo b/Doc/Zsh/mod_zpty.yo
index 3ca031c01..ba59e1783 100644
--- a/Doc/Zsh/mod_zpty.yo
+++ b/Doc/Zsh/mod_zpty.yo
@@ -66,8 +66,8 @@ read matches the var(pattern), even in the non-blocking case.  The return
 status is zero if the string read matches the pattern, or if the command
 has exited but at least one character could still be read.  If the option
 tt(-m) is present, the return status is zero only if the pattern matches.
-As of this writing, a maximum of one megabyte of output can be consumed
-this way; if a full megabyte is read without matching the pattern, the
+As of this writing, a maximum of one mebibyte of output can be consumed
+this way; if a full mebibyte is read without matching the pattern, the
 return status is non-zero.
 
 In all cases, the return status is non-zero if nothing could be read, and
diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index 36c1ae4c2..6c305da34 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -1473,7 +1473,7 @@ is specified with no command.  Defaults to tt(more).
 vindex(REPORTMEMORY)
 item(tt(REPORTMEMORY))(
 If nonnegative, commands whose maximum resident set size (roughly
-speaking, main memory usage) in kilobytes is greater than this
+speaking, main memory usage) in kibibytes is greater than this
 value have timing statistics reported.  The format used to output
 statistics is the value of the tt(TIMEFMT) parameter, which is the same
 as for the tt(REPORTTIME) variable and the tt(time) builtin; note that
@@ -1603,12 +1603,12 @@ sitem(tt(%E))(Elapsed time in seconds.)
 sitem(tt(%P))(The CPU percentage, computed as
 100*(tt(%U)PLUS()tt(%S))/tt(%E).)
 sitem(tt(%W))(Number of times the process was swapped.)
-sitem(tt(%X))(The average amount in (shared) text space used in kilobytes.)
+sitem(tt(%X))(The average amount in (shared) text space used in kibibytes.)
 sitem(tt(%D))(The average amount in (unshared) data/stack space used in
-kilobytes.)
-sitem(tt(%K))(The total space used (tt(%X)PLUS()tt(%D)) in kilobytes.)
+kibibytes.)
+sitem(tt(%K))(The total space used (tt(%X)PLUS()tt(%D)) in kibibytes.)
 sitem(tt(%M))(The  maximum memory the process had in use at any time in
-kilobytes.)
+kibibytes.)
 sitem(tt(%F))(The number of major page faults (page needed to be brought
 from disk).)
 sitem(tt(%R))(The number of minor page faults.)
diff --git a/Etc/BUGS b/Etc/BUGS
index 49e6a5c34..28e4f2b81 100644
--- a/Etc/BUGS
+++ b/Etc/BUGS
@@ -73,3 +73,7 @@ interactive and the subshell is the foreground job.  The USEZLE option is
 always turned off in subshells, for reasons lost to history.  There is a
 related, probably obsolete, vared special case for $TERM set to "emacs".
 ------------------------------------------------------------------------
+47634: missing rlim_t overflow checks in limit/ulimit limit arguments
+Integer values corresponding to the values of RLIM_INFINITY, RLIM_SAVED_MAX,
+RLIM_SAVED_CUR should probably be rejected as well (47639)
+------------------------------------------------------------------------
diff --git a/NEWS b/NEWS
index d05e8b64f..9d218153b 100644
--- a/NEWS
+++ b/NEWS
@@ -11,7 +11,12 @@ The zsh/system module's `zsystem flock` command learnt an -i option to
 set the wait interval used with -t. Additionally, -t now supports
 fractional seconds.
 
+The `limit` builtin accepts more units for resource limits, now shared
+with the `ulimit` builtin allowing limit values to be specified.
+`limit -s somelimit` now reports the shell process' own value of the limit.
 The `limit` and `unlimit` builtins are now available again in csh emulation.
+A few more improvements to the resource limit handling interfaces and
+documentation have been made.
 
 Changes from 5.7.1-test-3 to 5.8
 --------------------------------
diff --git a/Src/Builtins/rlimits.c b/Src/Builtins/rlimits.c
index 5f9c84b0f..b6568d956 100644
--- a/Src/Builtins/rlimits.c
+++ b/Src/Builtins/rlimits.c
@@ -55,7 +55,8 @@ typedef struct resinfo_T {
  * 1. Add zsh_LIMIT_PRESENT(RLIMIT_XXX) in configure.ac.
  * 2. Add an entry for RLIMIT_XXX to known_resources[].
  *    Make sure the option letter (resinto_T.opt) is unique.
- * 3. Build zsh and run the test B12rlimit.ztst.
+ * 3. Add entry in documentation for the limit and ulimit builtins
+ * 4. Build zsh and run the test B12rlimit.ztst.
  */
 static const resinfo_T known_resources[] = {
     {RLIMIT_CPU, "cputime", ZLIMTYPE_TIME, 1,
@@ -109,12 +110,23 @@ static const resinfo_T known_resources[] = {
 		'i', "pending signals"},
 # endif
 # ifdef HAVE_RLIMIT_MSGQUEUE
-    {RLIMIT_MSGQUEUE, "msgqueue", ZLIMTYPE_NUMBER, 1,
+    {RLIMIT_MSGQUEUE, "msgqueue", ZLIMTYPE_MEMORY, 1,
 		'q', "bytes in POSIX msg queues"},
 # endif
 # ifdef HAVE_RLIMIT_NICE
+    /*
+     * Not max niceness. On Linux ranges [1..40] for that RLIM translates
+     * to [-19..20] meaning that for instance setting it to 20 means
+     * processes can lower their niceness as far down as -1 and with the
+     * default of 0, unprivileged processes can't lower their niceness.
+     *
+     * Increasing niceness is always possible.
+     *
+     * "max nice priority" is the wording used in /proc/self/limits on
+     * Linux.
+     */
     {RLIMIT_NICE, "nice", ZLIMTYPE_NUMBER, 1,
-		'e', "max nice"},
+		'e', "max nice priority"},
 # endif
 # ifdef HAVE_RLIMIT_RTPRIO
     {RLIMIT_RTPRIO, "rt_priority", ZLIMTYPE_NUMBER, 1,
@@ -122,7 +134,7 @@ static const resinfo_T known_resources[] = {
 # endif
 # ifdef HAVE_RLIMIT_RTTIME
     {RLIMIT_RTTIME, "rt_time", ZLIMTYPE_MICROSECONDS, 1,
-		'N', "rt cpu time (microseconds)"},
+		'R', "rt cpu time slice (microseconds)"},
 # endif
     /* BSD */
 # ifdef HAVE_RLIMIT_SBSIZE
@@ -192,9 +204,9 @@ set_resinfo(void)
 	if (!resinfo[i]) {
 	    /* unknown resource */
 	    resinfo_T *info = (resinfo_T *)zshcalloc(sizeof(resinfo_T));
-	    char *buf = (char *)zalloc(12);
+	    char *buf = (char *)zalloc(8 /* UNKNOWN- */ + 3 /* digits */ + 1 /* '\0' */);
 	    snprintf(buf, 12, "UNKNOWN-%d", i);
-	    info->res = - 1;	/* negative value indicates "unknown" */
+	    info->res = -1;	/* negative value indicates "unknown" */
 	    info->name = buf;
 	    info->type = ZLIMTYPE_UNKNOWN;
 	    info->unit = 1;
@@ -255,38 +267,204 @@ printrlim(rlim_t val, const char *unit)
 # endif /* RLIM_T_IS_QUAD_T */
 }
 
+/*
+ * Parse a string into the corresponding value based on the limit type
+ *
+ *   ZLIMTYPE_UNKNOWN / ZLIMTYPE_NUMBER:
+ *     decimal integer without sign only: raw value
+ *
+ *   ZLIMTYPE_TIME / ZLIMTYPE_MICROSECONDS:
+ *     <decimal> only gives seconds or microseconds depending on type
+ *     or:
+ *     [[hour:]min:]sec[.usec]
+ *     or:
+ *     <decimal> with h (hour), m (min), s (sec), ms, us suffix.
+ *
+ *   ZLIMTYPE_MEMORY:
+ *     <decimal> without suffix interpreted as KiB by "limit" (except for
+ *     RLIMIT_MSGQUEUE, see below) and based on resinfo.unit by "ulimit".
+ *
+ *     K/M/G/T/P/E suffix and same with iB suffix use 1024 factor
+ *     KB/MB/GB... use 1000 factor.
+ *
+ *     B for bytes (avoids that mess about default units).
+ *
+ * All suffixes are case insensitive.
+ *
+ */
+
 /**/
 static rlim_t
-zstrtorlimt(const char *s, char **t, int base)
+zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 {
     rlim_t ret = 0;
+    const char *orig = s;
+    enum zlimtype type = resinfo[lim]->type;
+    *err = NULL;
 
-    if (strcmp(s, "unlimited") == 0) {
-	if (t)
-	    *t = (char *) s + 9;
+    if (strcmp(s, "unlimited") == 0)
 	return RLIM_INFINITY;
+
+    for (; *s >= '0' && *s <= '9'; s++)
+	ret = ret * 10 + *s - '0';
+
+    if (s == orig) {
+	*err = "decimal integer expected";
+	return 0;
+    }
+
+    if (lim >= RLIM_NLIMITS ||
+	type == ZLIMTYPE_NUMBER ||
+	type == ZLIMTYPE_UNKNOWN) {
+	/*
+	 * pure numeric resource -- only a straight decimal number is
+	 * permitted.
+	 */
+	if (*s) {
+	    *err = "limit must be a decimal integer";
+	    return 0;
+	}
+    }
+    else if (type == ZLIMTYPE_TIME ||
+	     type == ZLIMTYPE_MICROSECONDS) {
+	if (*s) {
+	    int divisor = 1;
+	    int factor = 1;
+	    switch (*s++) {
+
+	    case 'm': /* m for minute or ms for milisecond */
+	    case 'M':
+		if (*s == 's' || *s == 'S') {
+		    s++;
+		    divisor = 1000;
+		}
+		else {
+		    factor = 60;
+		}
+		break;
+
+	    case 'h':
+	    case 'H':
+		factor = 3600;
+		break;
+
+	    case 's':
+	    case 'S':
+		break;
+
+	    case 'u':
+	    case 'U':
+		divisor = 1000000;
+		if (*s == 's' || *s == 'S')
+		    s++;
+		break;
+
+	    case ':':
+		do {
+		    int more = 0;
+		    while (*s >= '0' && *s <= '9') {
+			more = more * 10 + (*s - '0');
+			s++;
+		    }
+		    ret = ret * 60 + more;
+		} while (*s == ':' && ++s);
+		if (*s == '.')
+		    s++;
+		    /* fallthrough */
+		else
+		    break;
+	    case '.':
+		if (type == ZLIMTYPE_MICROSECONDS) {
+		    int frac;
+		    for (frac = 0; *s >= '0' && *s <= '9'; s++) {
+			if (divisor < 1000000) {
+			    /* ignore digits past the 6th */
+			    divisor *= 10;
+			    frac = frac * 10 + (*s - '0');
+			}
+		    }
+		    ret = ret * divisor + frac;
+		}
+		else {
+		    /* fractional part ignored */
+		    while (*s >= '0' && *s <= '9')
+			s++;
+		}
+		break;
+	    default:
+		*err = "invalid time specification";
+		return 0;
+	    }
+
+	    if (*s) {
+		*err = "invalid time specification";
+		return 0;
+	    }
+
+	    ret *= factor;
+	    if (type == ZLIMTYPE_MICROSECONDS)
+		ret *= 1000000 / divisor;
+	    else
+		ret /= divisor;
+	}
+    }
+    else {
+	/*
+	 * memory-type resource
+	 */
+	if (*s) {
+	    if (*s == 'b' || *s == 'B')
+		s++;
+	    else {
+		const char *suffix = "kKmMgGtTpPeE";
+		char *offset;
+
+		if ((offset = strchr(suffix, *s))) {
+		    s++;
+		    if (*s == 'b' || *s == 'B') {
+			/* KB == 1000 */
+			const char *p;
+			for (p = suffix; p <= offset; p += 2)
+			    ret *= 1000;
+			s++;
+		    }
+		    else {
+			/* K/KiB == 1024 */
+			if ((s[0] == 'i' || s[0] == 'I') &&
+			    (s[1] == 'b' || s[1] == 'B'))
+			    s += 2;
+			ret <<= ((offset - suffix) / 2 + 1) * 10;
+		    }
+		}
+	    }
+	    if (*s) {
+		*err = "invalid unit";
+		return 0;
+	    }
+	}
+	else {
+	    if (ulimit)
+		ret *= resinfo[lim]->unit;
+	    else
+#ifdef HAVE_RLIMIT_MSGQUEUE
+		if (lim != RLIMIT_MSGQUEUE)
+		    /*
+		     * Historical quirk. In tcsh's limit (and bash's and mksh's
+		     * ulimit, but not ksh93), that limit expects bytes instead
+		     * of kibibytes and earlier versions of zsh were treating
+		     * it as a ZLIMTYPE_NUMBER.
+		     *
+		     * We still want to treat it as ZLIMTYPE_MEMORY and accept
+		     * KMG... suffixes as it is a number of bytes.
+		     *
+		     * But we carry on taking the value as a number of *bytes*
+		     * in the "limit" builtin for backward compatibility and
+		     * compatibility with tcsh.
+		     */
+#endif
+		    ret *= 1024;
+	}
     }
-# if defined(RLIM_T_IS_QUAD_T) || defined(RLIM_T_IS_LONG_LONG) || defined(RLIM_T_IS_UNSIGNED)
-    if (!base) {
-	if (*s != '0')
-	    base = 10;
-	else if (*++s == 'x' || *s == 'X')
-	    base = 16, s++;
-	else
-	    base = 8;
-    } 
-    if (base <= 10)
-	for (; *s >= '0' && *s < ('0' + base); s++)
-	    ret = ret * base + *s - '0';
-    else
-	for (; idigit(*s) || (*s >= 'a' && *s < ('a' + base - 10))
-	     || (*s >= 'A' && *s < ('A' + base - 10)); s++)
-	    ret = ret * base + (idigit(*s) ? (*s - '0') : (*s & 0x1f) + 9);
-    if (t)
-	*t = (char *)s;
-# else /* !RLIM_T_IS_QUAD_T && !RLIM_T_IS_LONG_LONG && !RLIM_T_IS_UNSIGNED */
-    ret = zstrtol(s, t, base);
-# endif /* !RLIM_T_IS_QUAD_T && !RLIM_T_IS_LONG_LONG && !RLIM_T_IS_UNSIGNED */
     return ret;
 }
 
@@ -317,25 +495,42 @@ showlimitvalue(int lim, rlim_t val)
 	       resinfo[lim]->type == ZLIMTYPE_UNKNOWN)
 	printrlim(val, "\n");	/* pure numeric resource */
     else {
-	/* memory resource -- display with `k' or `M' modifier */
-	if (val >= 1024L * 1024L)
-	    printrlim(val/(1024L * 1024L), "MB\n");
+	/*
+	 * memory resource -- display with KiB/MiB... for exact
+	 * multiples of those units
+	 */
+	const char *units = "KMGTPE";
+	rlim_t v = val;
+	int n = 0;
+	while (units[n] && (v & 1023) == 0 && v >> 10) {
+	    n++;
+	    v >>= 10;
+	}
+	if (n) {
+	    char suffix[] = "XiB\n";
+	    *suffix = units[n-1];
+	    printrlim(v, suffix);
+	}
 	else
-	    printrlim(val/1024L, "kB\n");
+	    printrlim(val, "B\n");
     }
 }
 
-/* Display resource limits.  hard indicates whether `hard' or `soft'  *
- * limits should be displayed.  lim specifies the limit, or may be -1 *
- * to show all.                                                       */
+/*
+ * Display resource limits.  hard indicates whether `hard' or `soft'
+ * limits should be displayed.  lim specifies the limit, or may be -1
+ * to show all.  If `shell' is non-zero, the limits in place for the
+ * shell process retrieved with getrlimits() are shown.
+ */
 
 /**/
 static int
-showlimits(char *nam, int hard, int lim)
+showlimits(char *nam, int hard, int lim, int shell)
 {
     int rt;
+    int ret = 0;
 
-    if (lim >= RLIM_NLIMITS)
+    if (shell || lim >= RLIM_NLIMITS)
     {
 	/*
 	 * Not configured into the shell.  Ask the OS
@@ -357,17 +552,40 @@ showlimits(char *nam, int hard, int lim)
     else
     {
 	/* main loop over resource types */
-	for (rt = 0; rt != RLIM_NLIMITS; rt++)
-	    showlimitvalue(rt, (hard) ? limits[rt].rlim_max :
-			   limits[rt].rlim_cur);
+	for (rt = 0; rt != RLIM_NLIMITS; rt++) {
+	    struct rlimit vals, *plim;
+	    if (shell) {
+		/*
+		 * FIXME: this is dead code as at the moment, there is no API
+		 * for the user can request all limits *of the shell* be
+		 * displayed.
+		 *
+		 * Should we add a "limit -s -a" or "limit -s all"?
+		 */
+		plim = &vals;
+		if (getrlimit(rt, plim) < 0)
+		{
+		    zwarnnam(nam, "can't read \"%s\" limit: %e", resinfo[rt]->name, errno);
+		    ret = 1;
+		    continue;
+		}
+	    }
+	    else
+		plim = &(limits[rt]);
+
+	    showlimitvalue(rt, (hard) ? plim->rlim_max :
+			   plim->rlim_cur);
+	}
     }
 
-    return 0;
+    return ret;
 }
 
-/* Display a resource limit, in ulimit style.  lim specifies which   *
- * limit should be displayed, and hard indicates whether the hard or *
- * soft limit should be displayed.                                   */
+/*
+ * Display a resource limit, in ulimit style.  lim specifies which
+ * limit should be displayed, and hard indicates whether the hard or
+ * soft limit should be displayed.
+ */
 
 /**/
 static int
@@ -394,12 +612,12 @@ printulimit(char *nam, int lim, int hard, int head)
 	if (lim < RLIM_NLIMITS) {
 	    const resinfo_T *info = resinfo[lim];
 	    if (info->opt == 'N')
-		printf("-N %2d: %-29s", lim, info->descr);
+		printf("-N %2d: %-32s ", lim, info->descr);
 	    else
-		printf("-%c: %-32s", info->opt, info->descr);
+		printf("-%c: %-35s ", info->opt, info->descr);
 	}
 	else
-	    printf("-N %2d: %-29s", lim, "");
+	    printf("-N %2d: %-32s ", lim, "");
     }
     /* display the limit */
     if (limit == RLIM_INFINITY)
@@ -511,16 +729,18 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
     rlim_t val;
     int ret = 0;
 
-    hard = OPT_ISSET(ops,'h');
-    if (OPT_ISSET(ops,'s') && !*argv)
+    hard = OPT_ISSET(ops, 'h');
+    if (OPT_ISSET(ops, 's') && !*argv)
 	return setlimits(NULL);
     /* without arguments, display limits */
     if (!*argv)
-	return showlimits(nam, hard, -1);
+	return showlimits(nam, hard, -1, OPT_ISSET(ops, 's'));
     while ((s = *argv++)) {
 	/* Search for the appropriate resource name.  When a name matches (i.e. *
 	 * starts with) the argument, the lim variable changes from -1 to the   *
 	 * number of the resource.  If another match is found, lim goes to -2.  */
+	char *err;
+
 	if (idigit(*s))
 	{
 	    lim = (int)zstrtol(s, NULL, 10);
@@ -543,61 +763,12 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
 	}
 	/* without value for limit, display the current limit */
 	if (!(s = *argv++))
-	    return showlimits(nam, hard, lim);
-	if (lim >= RLIM_NLIMITS)
-	{
-	    val = zstrtorlimt(s, &s, 10);
-	    if (*s)
-	    {
-		/* unknown limit, no idea how to scale */
-		zwarnnam(nam, "unknown scaling factor: %s", s);
-		return 1;
-	    }
-	}
-	else if (resinfo[lim]->type == ZLIMTYPE_TIME) {
-	    /* time-type resource -- may be specified as seconds, or minutes or *
-	     * hours with the `m' and `h' modifiers, and `:' may be used to add *
-	     * together more than one of these.  It's easier to understand from *
-	     * the code:                                                        */
-	    val = zstrtorlimt(s, &s, 10);
-	    if (*s) {
-		if ((*s == 'h' || *s == 'H') && !s[1])
-		    val *= 3600L;
-		else if ((*s == 'm' || *s == 'M') && !s[1])
-		    val *= 60L;
-		else if (*s == ':')
-		    val = val * 60 + zstrtorlimt(s + 1, &s, 10);
-		else {
-		    zwarnnam(nam, "unknown scaling factor: %s", s);
-		    return 1;
-		}
-	    }
-	} else if (resinfo[lim]->type == ZLIMTYPE_NUMBER ||
-		   resinfo[lim]->type == ZLIMTYPE_UNKNOWN ||
-		   resinfo[lim]->type == ZLIMTYPE_MICROSECONDS) {
-	    /* pure numeric resource -- only a straight decimal number is
-	    permitted. */
-	    char *t = s;
-	    val = zstrtorlimt(t, &s, 10);
-	    if (s == t) {
-		zwarnnam(nam, "limit must be a number");
-		return 1;
-	    }
-	} else {
-	    /* memory-type resource -- `k', `M' and `G' modifiers are *
-	     * permitted, meaning (respectively) 2^10, 2^20 and 2^30. */
-	    val = zstrtorlimt(s, &s, 10);
-	    if (!*s || ((*s == 'k' || *s == 'K') && !s[1])) {
-		if (val != RLIM_INFINITY)
-		    val *= 1024L;
-	    } else if ((*s == 'M' || *s == 'm') && !s[1])
-		val *= 1024L * 1024;
-	    else if ((*s == 'G' || *s == 'g') && !s[1])
-		val *= 1024L * 1024 * 1024;
-	    else {
-		zwarnnam(nam, "unknown scaling factor: %s", s);
-		return 1;
-	    }
+	    return showlimits(nam, hard, lim, OPT_ISSET(ops, 's'));
+
+	val = zstrtorlimt(s, lim, 0, &err);
+	if (err) {
+	    zwarnnam(nam, err);
+	    return 1;
 	}
 	if (do_limit(nam, lim, val, hard, !hard, OPT_ISSET(ops, 's')))
 	    ret++;
@@ -821,14 +992,12 @@ bin_ulimit(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		    limit = vals.rlim_max;
 		}
 	    } else {
-		limit = zstrtorlimt(*argv, &eptr, 10);
-		if (*eptr) {
-		    zwarnnam(name, "invalid number: %s", *argv);
+		char *err;
+		limit = zstrtorlimt(*argv, res, 1, &err);
+		if (err) {
+		    zwarnnam(name, "%s: %s", *argv, err);
 		    return 1;
 		}
-		/* scale appropriately */
-		if (res < RLIM_NLIMITS)
-		    limit *= resinfo[res]->unit;
 	    }
 	    if (do_limit(name, res, limit, hard, soft, 1))
 		ret++;
diff --git a/Test/B12limit.ztst b/Test/B12limit.ztst
index 48d33e6e3..7c45deb8f 100644
--- a/Test/B12limit.ztst
+++ b/Test/B12limit.ztst
@@ -26,3 +26,160 @@ F:report this to zsh-workers mailing list.
   }
 0:check if limit option letters are unique
 
+  if sh -c 'ulimit -f 2048' > /dev/null 2>&1; then
+    (
+      set -o braceccl -o pipefail
+      list=(1b 1{kmgtpe}{,b,ib})
+      for cmd in "limit filesize" ulimit; do
+	for l in $list $list:u; do
+	  $=cmd $l &&
+	    limit filesize &&
+	    ulimit || exit
+	done
+      done | sed 'N;s/\n/ /;s/  */ /g'
+    )
+  else
+    ZTST_skip='Cannot set the filesize limit on this system'
+  fi
+0:filesize suffixes with limit and ulimit
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+
+  if sh -c 'ulimit -t 3600' > /dev/null 2>&1; then
+  (
+    set -o pipefail
+    list=(1h 30m 20s 30 1:23:45.123456 2:23 56.4)
+    for cmd in "limit cputime" "ulimit -t"; do
+      for l in $list ${(MU)list:#*[a-z]*}; do
+        $=cmd $l &&
+	  limit cputime &&
+	  ulimit -t || exit
+      done
+    done | sed 'N;s/\n/ /;s/  */ /g'
+  )
+  else
+    ZTST_skip='Cannot set the cputime limit on this system'
+  fi
+0:time limit formats
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 0:00:30 30
+>cputime 1:23:45 5025
+>cputime 0:02:23 143
+>cputime 0:00:56 56
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 0:00:30 30
+>cputime 1:23:45 5025
+>cputime 0:02:23 143
+>cputime 0:00:56 56
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+
+  ulimit 1Kite
+  ulimit 1D
+  ulimit 1s
+  ulimit 1MBA
+  limit cputime 1k
+  limit cputime 1:0s
+  limit cputime 1ss
+  limit cputime 1msx
+  limit cputime 1.0s
+  limit cputime .1
+  limit descriptors 1k
+  limit descriptors 1h
+  limit descriptors 1:0
+1:invalid limit input
+?(eval):ulimit:1: 1Kite: invalid unit
+?(eval):ulimit:2: 1D: invalid unit
+?(eval):ulimit:3: 1s: invalid unit
+?(eval):ulimit:4: 1MBA: invalid unit
+?(eval):limit:5: invalid time specification
+?(eval):limit:6: invalid time specification
+?(eval):limit:7: invalid time specification
+?(eval):limit:8: invalid time specification
+?(eval):limit:9: invalid time specification
+?(eval):limit:10: decimal integer expected
+?(eval):limit:11: limit must be a decimal integer
+?(eval):limit:12: limit must be a decimal integer
+?(eval):limit:13: limit must be a decimal integer
-- 
2.25.1




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2020-11-26 20:14   ` [PATCH] ulimit option completions using ulimit -a output Stephane Chazelas
@ 2020-11-27  7:13     ` Stephane Chazelas
  2020-11-27  8:15       ` Felipe Contreras
  2020-11-27 12:19       ` Oliver Kiddle
  2021-04-13 14:35     ` Daniel Shahaf
  1 sibling, 2 replies; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-27  7:13 UTC (permalink / raw)
  To: Zsh hackers list

2020-11-26 20:14:31 +0000, Stephane Chazelas:
[...]
> The problem is that the list of ulimit options is system
> dependent, so I propose the improvement below which uses the
> output of ulimit -a to build the list of completions.
[...]
> +++ b/Completion/Zsh/Command/_ulimit
[...]
>    '-S[set soft and hard limits (with -H)]' \
[...]
> +  '-N+[specify limit by number]:limit number' \
[...]
> +  '*:file size (blocks)'
[...]

Sorry, should have been

'-N+[specify limit by number]:limit number::limit value'

Otherwise you'd get "Completing file size (blocks)" when
pressing Tab after "ulimit -N 15 ".

Now I need to figure out how to amend a commit that is not the
latest...

By the way, should I cache that list of options retrieved from
the output of "ulimit -a"? After all, it's not going to change
from one run to the next. But then again, a $(ulimit -a) command
substitution is going to be fast and caching uses up memory. Do
you have recommendations as to when to cache and when not to?

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2020-11-27  7:13     ` Stephane Chazelas
@ 2020-11-27  8:15       ` Felipe Contreras
  2020-11-27 12:19       ` Oliver Kiddle
  1 sibling, 0 replies; 30+ messages in thread
From: Felipe Contreras @ 2020-11-27  8:15 UTC (permalink / raw)
  To: Zsh hackers list

On Fri, Nov 27, 2020 at 1:14 AM Stephane Chazelas <stephane@chazelas.org> wrote:

> Now I need to figure out how to amend a commit that is not the
> latest...

git rebase --interactive
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

Or you can find the commit you want to fix, then put the id in the
title of your patch commit (e.g. "fixup! 9686f839c3"), then do "git
rebase --autosquash 9686f839c3~" (one commit before the one you want
to fix).

Cheers.

-- 
Felipe Contreras



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2020-11-27  7:13     ` Stephane Chazelas
  2020-11-27  8:15       ` Felipe Contreras
@ 2020-11-27 12:19       ` Oliver Kiddle
  2021-03-27 21:25         ` Lawrence Velázquez
  1 sibling, 1 reply; 30+ messages in thread
From: Oliver Kiddle @ 2020-11-27 12:19 UTC (permalink / raw)
  To: Zsh hackers list

Stephane Chazelas wrote:
> By the way, should I cache that list of options retrieved from
> the output of "ulimit -a"? After all, it's not going to change

I wouldn't bother. It's a shell builtin, everything it outputs is
straight from memory so the only cost is a single fork. No delay is
discernable.

> from one run to the next. But then again, a $(ulimit -a) command
> substitution is going to be fast and caching uses up memory. Do
> you have recommendations as to when to cache and when not to?

Mostly it comes down to what seems sensible in a particular case. The
main criteria being if a delay is noticable. But it would also depend
whether it is the type of command that is likely used repeatedly in a
session (which ulimit probably isn't much). There's also the choice
of just cache in a variable in memory vs. using the cache functions
to store it in a file. I'm more inclined towards that when it is very
large.

Oliver



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2020-11-26 20:58   ` [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
@ 2020-11-27 16:39     ` Daniel Shahaf
  2020-11-27 20:13       ` Stephane Chazelas
  0 siblings, 1 reply; 30+ messages in thread
From: Daniel Shahaf @ 2020-11-27 16:39 UTC (permalink / raw)
  To: Zsh hackers list

Stephane Chazelas wrote on Thu, Nov 26, 2020 at 20:58:19 +0000:
> +++ b/Src/Builtins/rlimits.c
> @@ -255,38 +267,204 @@ printrlim(rlim_t val, const char *unit)
>  # endif /* RLIM_T_IS_QUAD_T */
>  }
>  
> +/*
> + * Parse a string into the corresponding value based on the limit type
> + *
> + *   ZLIMTYPE_UNKNOWN / ZLIMTYPE_NUMBER:
> + *     decimal integer without sign only: raw value
> + *
> + *   ZLIMTYPE_TIME / ZLIMTYPE_MICROSECONDS:
> + *     <decimal> only gives seconds or microseconds depending on type
> + *     or:
> + *     [[hour:]min:]sec[.usec]
> + *     or:
> + *     <decimal> with h (hour), m (min), s (sec), ms, us suffix.
> + *
> + *   ZLIMTYPE_MEMORY:
> + *     <decimal> without suffix interpreted as KiB by "limit" (except for
> + *     RLIMIT_MSGQUEUE, see below) and based on resinfo.unit by "ulimit".
> + *
> + *     K/M/G/T/P/E suffix and same with iB suffix use 1024 factor
> + *     KB/MB/GB... use 1000 factor.
> + *
> + *     B for bytes (avoids that mess about default units).
> + *
> + * All suffixes are case insensitive.
> + *
> + */
> +
>  /**/
>  static rlim_t
> -zstrtorlimt(const char *s, char **t, int base)
> +zstrtorlimt(const char *s, int lim, int ulimit, char **err)

Please specify in the docstring the types, values, and meanings of the
formal parameters.

>  {
>      rlim_t ret = 0;
> +    const char *orig = s;
> +    enum zlimtype type = resinfo[lim]->type;
> +    *err = NULL;
> +    else if (type == ZLIMTYPE_TIME ||
> +	     type == ZLIMTYPE_MICROSECONDS) {
> +	    default:
> +		*err = "invalid time specification";

Could you arrange for the invalid value to be included in the error
message, as in «zwarnnam(nam, "invalid time specification: %s", foo)»?
That tends to be more user-friendly, particularly when the call stack is
deep.

> +		return 0;
> +	    }
> +
> +	    if (*s) {
> +		*err = "invalid time specification";
> +		return 0;
> +	    }
> +
> +	    ret *= factor;
> +	    if (type == ZLIMTYPE_MICROSECONDS)
> +		ret *= 1000000 / divisor;
> +	    else
> +		ret /= divisor;
> +	}
> +    }
> @@ -543,61 +763,12 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
> +	val = zstrtorlimt(s, lim, 0, &err);
> +	if (err) {
> +	    zwarnnam(nam, err);

This should be zwarnnam(nam, "%s", err), otherwise any percent signs in
«err» would cause breakage.

> +	    return 1;
>  	}
>  	if (do_limit(nam, lim, val, hard, !hard, OPT_ISSET(ops, 's')))
>  	    ret++;

Cheers,

Daniel
(haven't done a full review; just ran into thils in skimming)



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-25  6:44   ` Stephane Chazelas
@ 2020-11-27 17:16     ` Daniel Shahaf
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Shahaf @ 2020-11-27 17:16 UTC (permalink / raw)
  To: Zsh hackers list

Stephane Chazelas wrote on Wed, Nov 25, 2020 at 06:44:43 +0000:
> 2020-11-25 00:35:12 +0000, Daniel Shahaf:
> [...]
> > > 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.
> [...]
> 
> Hi Daniel,
> 
> thanks for having looking into it.
> 
> They're not independent, except maybe for the "limit"/"unlimit"
> now back in csh which is a 5 code line change.
> 

*nod*

> Chunks of the code and doc have been rewritten to address
> those. Separating out the changes would mean rewriting the code
> several times which would be more effort for me and reviewers,
> mostly wasted if we're only keeping the last iteration.

How so?  In v2 2/2's log message I see, for starters, several bullet
points about documentation-only bugs, and in the .c diff the
showlimitvalue() stands out as _prima facie_ independent of the rest.

I don't see how _splitting_ those involves a _rewrite_.  Splitting is
fairly easy,¹ and I don't think it'd be a "waste" of time.  On the
contrary, I think it'd be a O(1) effort for O(N) gain (split it once and
then, even years from now, anyone who reviews the history will have a
easier time).  Moreover, smaller diff can help ensure that each change
is tested and help catch unintentional behaviour changes (always a risk
in refactorings).

I do agree that a single logical change that happens to fix a dozen
interdependent bugs needn't be split, but I don't see how that's the
case here.  Enlighten me ☺

> But maybe before looking in detail at the code, we can discuss
> whether the change in API makes sense.

Seems to make sense, going by the log message.  Really, the only part
that jumped out at me was changing the behaviour of "KB" on input from
error to "1000 bytes".

Cheers,

Daniel

¹ Stephane: I elaborated offlist recently on splitting.



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-26 17:23       ` Stephane Chazelas
@ 2020-11-27 18:24         ` Jun. T
  2020-11-27 18:34           ` Daniel Shahaf
  2020-11-27 20:46           ` Stephane Chazelas
  0 siblings, 2 replies; 30+ messages in thread
From: Jun. T @ 2020-11-27 18:24 UTC (permalink / raw)
  To: zsh-workers


> 2020/11/27 2:23, Stephane Chazelas <stephane@chazelas.org> wrote:
> 
> 2020-11-27 00:22:05 +0900, Jun. T:
> 
>> Zsh only need to check whether the new limit user wants to set is
>> within the range of rlim_t.
> 
> Yes, but how do you determine that range? Should we not also
> reject 18446744073709551615 as out-of-range on systems where
> it's RLIM_INFINITY since it's not preventing file sizes to get
> past 18446744073709551615 for instance.

I guess you mean we should reject RLIM_INFINITY, and yes I agree with it.
How about the patch below (to your v2)?

# Even if you accept this patch, maybe better to commit it separately after
# your patch (which may be separated into parts) for finer granularity.

Jun


diff --git a/Src/Builtins/rlimits.c b/Src/Builtins/rlimits.c
index b6568d956..69174fe8e 100644
--- a/Src/Builtins/rlimits.c
+++ b/Src/Builtins/rlimits.c
@@ -297,7 +297,7 @@ printrlim(rlim_t val, const char *unit)
 static rlim_t
 zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 {
-    rlim_t ret = 0;
+    rlim_t ret = 0, tmp;
     const char *orig = s;
     enum zlimtype type = resinfo[lim]->type;
     *err = NULL;
@@ -305,8 +305,14 @@ zstrtorlimt(const char *s, int lim, int ulimit, char **err)
     if (strcmp(s, "unlimited") == 0)
 	return RLIM_INFINITY;
 
-    for (; *s >= '0' && *s <= '9'; s++)
-	ret = ret * 10 + *s - '0';
+    for (; *s >= '0' && *s <= '9'; s++) {
+	if ((tmp = ret * 10 + *s - '0') < ret) {
+	    *err = "limit out of range";
+	    return 0;
+	}
+	else
+	    ret = tmp;
+    }
 
     if (s == orig) {
 	*err = "decimal integer expected";
@@ -412,6 +418,7 @@ zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 	/*
 	 * memory-type resource
 	 */
+	rlim_t unit = 1;
 	if (*s) {
 	    if (*s == 'b' || *s == 'B')
 		s++;
@@ -425,7 +432,7 @@ zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 			/* KB == 1000 */
 			const char *p;
 			for (p = suffix; p <= offset; p += 2)
-			    ret *= 1000;
+			    unit *= 1000;
 			s++;
 		    }
 		    else {
@@ -433,7 +440,7 @@ zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 			if ((s[0] == 'i' || s[0] == 'I') &&
 			    (s[1] == 'b' || s[1] == 'B'))
 			    s += 2;
-			ret <<= ((offset - suffix) / 2 + 1) * 10;
+			unit <<= ((offset - suffix) / 2 + 1) * 10;
 		    }
 		}
 	    }
@@ -444,7 +451,7 @@ zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 	}
 	else {
 	    if (ulimit)
-		ret *= resinfo[lim]->unit;
+		unit = resinfo[lim]->unit;
 	    else
 #ifdef HAVE_RLIMIT_MSGQUEUE
 		if (lim != RLIMIT_MSGQUEUE)
@@ -462,8 +469,19 @@ zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 		     * compatibility with tcsh.
 		     */
 #endif
-		    ret *= 1024;
+		    unit = 1024;
 	}
+	if ((tmp = ret*unit) < ret) {
+	    *err = "limit out of range";
+	    return 0;
+	}
+	else
+	    ret = tmp;
+    }
+    if (ret == RLIM_INFINITY) {
+	/* RLIM_INFINITY can be specified only by the string "unlimited" */
+	*err = "limit out of range";
+	return 0;
     }
     return ret;
 }






^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-27 18:24         ` Jun. T
@ 2020-11-27 18:34           ` Daniel Shahaf
  2020-11-27 20:46           ` Stephane Chazelas
  1 sibling, 0 replies; 30+ messages in thread
From: Daniel Shahaf @ 2020-11-27 18:34 UTC (permalink / raw)
  To: zsh-workers

Jun. T wrote on Fri, 27 Nov 2020 18:24 +00:00:
> # Even if you accept this patch, maybe better to commit it separately after
> # your patch (which may be separated into parts) for finer granularity.

Reminder: if y'all think a topic branch will help organize this work,
don't hesitate to create one.  No need to ask ☺

Cheers,

Daniel



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2020-11-27 16:39     ` Daniel Shahaf
@ 2020-11-27 20:13       ` Stephane Chazelas
  2020-11-27 20:36         ` Daniel Shahaf
  2020-11-28  8:16         ` [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
  0 siblings, 2 replies; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-27 20:13 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

2020-11-27 16:39:49 +0000, Daniel Shahaf:
[...]
> Please specify in the docstring the types, values, and meanings of the
> formal parameters.

I've tried to address that in this v3 patch. Specifying the type
(as in char*, int...) seems redundant though and not generally
done in other functions, so I've left it out for now.

[...]
> Could you arrange for the invalid value to be included in the error
> message, as in «zwarnnam(nam, "invalid time specification: %s", foo)»?
> That tends to be more user-friendly, particularly when the call stack is
> deep.
[...]
> > +	    zwarnnam(nam, err);
> 
> This should be zwarnnam(nam, "%s", err), otherwise any percent signs in
> «err» would cause breakage.
[...]

Thanks. Both very good points. I had done that for ulimit, but
not limit. Should just be a matter of:

--- a/Src/Builtins/rlimits.c
+++ b/Src/Builtins/rlimits.c
@@ -767,7 +767,7 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
 
 	val = zstrtorlimt(s, lim, 0, &err);
 	if (err) {
-	    zwarnnam(nam, err);
+	    zwarnnam(nam, "%s: %s", s, err);
 	    return 1;
 	}
 	if (do_limit(nam, lim, val, hard, !hard, OPT_ISSET(ops, 's')))

(along with the test update)

From: Stephane Chazelas <stephane@chazelas.org>
Date: Thu, 26 Nov 2020 20:38:57 +0000
Subject: [PATCH] improve limit/ulimit API and documentation

A few issues addressed:

* 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` 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    | 125 +++++++++----
 Doc/Zsh/expn.yo        |  18 +-
 Doc/Zsh/mod_zpty.yo    |   4 +-
 Doc/Zsh/params.yo      |  10 +-
 Etc/BUGS               |   4 +
 NEWS                   |   5 +
 Src/Builtins/rlimits.c | 407 +++++++++++++++++++++++++++++------------
 Test/B12limit.ztst     | 157 ++++++++++++++++
 8 files changed, 563 insertions(+), 167 deletions(-)

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 2f1ccd8a5..5a2c51b0b 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1116,15 +1116,18 @@ cindex(resource limits)
 cindex(limits, resource)
 item(tt(limit) [ tt(-hs) ] [ var(resource) [ var(limit) ] ] ...)(
 Set or display resource limits.  Unless the tt(-s) flag is given,
-the limit applies only the children of the shell.  If tt(-s) is
-given without other arguments, the resource limits of the current
-shell is set to the previously set resource limits of the children.
+the limit applies only to child processes and executed commands, not the
+current shell process itself.
 
-If var(limit) is not specified, print the current limit placed
-on var(resource), otherwise
-set the limit to the specified value.  If the tt(-h) flag
-is given, use hard limits instead of soft limits.
-If no var(resource) is given, print all limits.
+If tt(-s) is given without other arguments, the resource limits of the
+current shell are set to the previously set resource limits of the
+children.
+
+If var(limit) is not specified, print the current limit placed on
+var(resource) (of the shell with tt(-s), of children without), otherwise
+set the limit to the specified value.  If the tt(-h) flag is given, use
+hard limits instead of soft limits. If no var(resource) is given, print
+all limits.
 
 When looping over multiple resources, the shell will abort immediately if
 it detects a badly formed argument.  However, if it fails to set a limit
@@ -1143,18 +1146,23 @@ sitem(tt(datasize))(Maximum data size (including stack) for each process.)
 sitem(tt(descriptors))(Maximum value for a file descriptor.)
 sitem(tt(filesize))(Largest single file allowed.)
 sitem(tt(kqueues))(Maximum number of kqueues allocated.)
+sitem(tt(maxfilelocks))(Maximum number of file locks.)
 sitem(tt(maxproc))(Maximum number of processes.)
-sitem(tt(maxpthreads))(Maximum number of threads per process.)
+sitem(tt(maxpthreads))(Maximum number of threads per uid (NetBSD/OpenBSD) or per process.)
 sitem(tt(memorylocked))(Maximum amount of memory locked in RAM.)
 sitem(tt(memoryuse))(Maximum resident set size.)
 sitem(tt(msgqueue))(Maximum number of bytes in POSIX message queues.)
+sitem(tt(nice))(Maximum nice priority.)
 sitem(tt(posixlocks))(Maximum number of POSIX locks per user.)
 sitem(tt(pseudoterminals))(Maximum number of pseudo-terminals.)
 sitem(tt(resident))(Maximum resident set size.)
+sitem(tt(rt_priority))(Maximum realtime priority.)
+sitem(tt(rt_time))(Maximum realtime CPU time slice.)
 sitem(tt(sigpending))(Maximum number of pending signals.)
 sitem(tt(sockbufsize))(Maximum size of all socket buffers.)
 sitem(tt(stacksize))(Maximum stack size for each process.)
 sitem(tt(swapsize))(Maximum amount of swap used.)
+sitem(tt(umtxp))(Maximum number of umtx shared locks.)
 sitem(tt(vmemorysize))(Maximum amount of virtual memory.)
 endsitem()
 
@@ -1169,14 +1177,42 @@ the limit anyway, and will report an error if this fails.  As the shell
 does not store such resources internally, an attempt to set the limit will
 fail unless the tt(-s) option is present.
 
-var(limit) is a number, with an optional scaling factor, as follows:
+If var(limit) is a decimal integer number without suffix, then for
+historical reasons and compatibility with csh where that command comes from,
+the unit will depend on the type of limit: microseconds for tt(rt_time),
+seconds for all other time limits, kibibytes (1024 bytes) for all limits that
+express a number of bytes (except tt(msgqueue)).
+
+Instead, to avoid confusion, a suffix (case insensitive) may be appended to
+the (integer only) decimal number to specify the unit:
 
 startsitem()
-sitem(var(n)tt(h))(hours)
-sitem(var(n)tt(k))(kilobytes (default))
-sitem(var(n)tt(m))(megabytes or minutes)
-sitem(var(n)tt(g))(gigabytes)
-sitem([var(mm)tt(:)]var(ss))(minutes and seconds)
+sitem(time limits)(
+startsitem()
+sitem(tt(h))(hours)
+sitem(tt(m))(minutes)
+sitem(tt(s))(seconds)
+sitem(tt(ms))(milliseconds)
+sitem(tt(us))(microseconds)
+sitem(tt([[var(h):]var(m):]var(s)[.var(usec)]))(all combined)
+endsitem()
+
+For instance a 2 millisecond limit of tt(rt_time), can be expressed as
+tt(2ms), tt(2000us), tt(0.002), tt(0:0:0.002) or tt(2000) without unit.)
+sitem(memory limits)(
+startsitem()
+sitem(tt(k, kib))(kibibytes (1024 bytes))
+sitem(tt(m, mib))(mibibytes)
+sitem(...)(and so on with tt(g), tt(t), tt(p), tt(e) for gibibytes,
+tebibytes, pebibytes and exbibytes respectively)
+sitem(tt(kb))(kilobytes (1,000 bytes))
+sitem(tt(mb))(megabytes (1,000,000 bytes))
+sitem(...)(and so on with tt(g), tt(t), tt(p), tt(e) for gigabytes,
+terabytes, petabytes and exabytes respectively.)
+sitem(tt(b))(em(byte) to set the limit with arbitrary precisions)
+endsitem())
+sitem(other limits)(other limits are assumed to be numerical and ony
+a decimal integer number is accepted.)
 endsitem()
 
 The tt(limit) command is not made available by default when the
@@ -2250,8 +2286,8 @@ together with the tt(-H) flag set both hard and soft limits.
 If no options are used, the file size limit (tt(-f)) is assumed.
 
 If var(limit) is omitted the current value of the specified resources are
-printed.  When more than one resource value is printed, the limit name and
-unit is printed before each value.
+printed (rounded down to the corresponding unit).  When more than one resource
+value is printed, the limit name and unit is printed before each value.
 
 When looping over multiple resources, the shell will abort immediately if
 it detects a badly formed argument.  However, if it fails to set a limit
@@ -2260,30 +2296,45 @@ for some other reason it will continue trying to set the remaining limits.
 Not all the following resources are supported on all systems.  Running
 tt(ulimit -a) will show which are supported.
 
+Whilst tt(limit) is the BSD/csh-style command, tt(ulimit) is the SysV/Korn
+shell equivalent (added later to zsh for compatibility with ksh). While
+tt(limit) in zsh sets the limits for child processes by default, tt(ulimit)
+sets them for both the shell and child processes, and reports the ones
+set for child processes.
+
+The same suffixes are recognised as for tt(limit), but bear in mind that
+default units when no suffix is specified vary between the two.
+
+Below, the corresponding tt(limit) keyword for each tt(ulimit) option is
+shown in parenthesis for reference:
+
 startsitem()
 sitem(tt(-a))(Lists all of the current resource limits.)
-sitem(tt(-b))(Socket buffer size in bytes LPAR()N.B. not kilobytes+RPAR())
-sitem(tt(-c))(512-byte blocks on the size of core dumps.)
-sitem(tt(-d))(Kilobytes on the size of the data segment.)
-sitem(tt(-f))(512-byte blocks on the size of files written.)
-sitem(tt(-i))(The number of pending signals.)
-sitem(tt(-k))(The number of kqueues allocated.)
-sitem(tt(-l))(Kilobytes on the size of locked-in memory.)
-sitem(tt(-m))(Kilobytes on the size of physical memory.)
-sitem(tt(-n))(open file descriptors.)
-sitem(tt(-p))(The number of pseudo-terminals.)
-sitem(tt(-q))(Bytes in POSIX message queues.)
+sitem(tt(-b))(Socket buffer size in bytes LPAR()N.B. not kibibytes+RPAR() (tt(sockbufsize)).)
+sitem(tt(-c))(512-byte blocks on the size of core dumps (tt(coredumpsize)).)
+sitem(tt(-d))(Kibibytes on the size of the data segment (tt(datasize)).)
+sitem(tt(-e))(nice priority (tt(nice)).)
+sitem(tt(-f))(512-byte blocks on the size of files written (tt(filesize)).)
+sitem(tt(-i))(The number of pending signals (tt(sigpending)).)
+sitem(tt(-k))(The number of kqueues allocated (tt(kqueues)).)
+sitem(tt(-l))(Kibibytes on the size of locked-in memory (tt(memorylocked)).)
+sitem(tt(-m))(Kibibytes on the size of physical memory (tt(resident)).)
+sitem(tt(-n))(open file descriptors (tt(descriptors)).)
+sitem(tt(-o))(umtx shared locks (tt(umtxp)).)
+sitem(tt(-p))(The number of pseudo-terminals (tt(pseudoterminals)).)
+sitem(tt(-q))(Bytes in POSIX message queues (tt(msgqueue)).)
 sitem(tt(-r))(Maximum real time priority.  On some systems where this
 is not available, such as NetBSD, this has the same effect as tt(-T)
-for compatibility with tt(sh).)
-sitem(tt(-s))(Kilobytes on the size of the stack.)
-sitem(tt(-T))(The number of simultaneous threads available to the user.)
-sitem(tt(-t))(CPU seconds to be used.)
-sitem(tt(-u))(The number of processes available to the user.)
-sitem(tt(-v))(Kilobytes on the size of virtual memory.  On some systems this
-refers to the limit called `address space'.)
-sitem(tt(-w))(Kilobytes on the size of swapped out memory.)
-sitem(tt(-x))(The number of locks on files.)
+for compatibility with tt(sh) (tt(rt_priority) / tt(maxpthreads)).)
+sitem(tt(-R))(realtime CPU time slice (tt(rt_time)).)
+sitem(tt(-s))(Kibibytes on the size of the stack (tt(stacksize)).)
+sitem(tt(-T))(The number of simultaneous threads available to the user (tt(maxpthreads)).)
+sitem(tt(-t))(CPU seconds to be used (tt(cputime)).)
+sitem(tt(-u))(The number of processes available to the user (tt(maxproc)).)
+sitem(tt(-v))(Kibibytes on the size of virtual memory.  On some systems this
+refers to the limit called em(address space) (tt(vmemorysize) / tt(addressspace)).)
+sitem(tt(-w))(Kibibytes on the size of swapped out memory (tt(swapsize)).)
+sitem(tt(-x))(The number of locks on files (tt(maxfilelocks)).)
 endsitem()
 
 A resource may also be specified by integer in the form `tt(-N)
diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index b3396721f..259c00a6c 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -2837,15 +2837,15 @@ exactly var(n) bytes in length.
 
 If this flag is directly followed by a em(size specifier) `tt(k)' (`tt(K)'),
 `tt(m)' (`tt(M)'), or `tt(p)' (`tt(P)') (e.g. `tt(Lk-50)') the check is
-performed with kilobytes, megabytes, or blocks (of 512 bytes) instead.
-(On some systems additional specifiers are available for gigabytes,
-`tt(g)' or `tt(G)', and terabytes, `tt(t)' or `tt(T)'.) If a size specifier
-is used a file is regarded as "exactly" the size if the file size rounded up
-to the next unit is equal to the test size.  Hence `tt(*LPAR()Lm1+RPAR())'
-matches files from 1 byte up to 1 Megabyte inclusive.  Note also that
-the set of files "less than" the test size only includes files that would
-not match the equality test; hence `tt(*LPAR()Lm-1+RPAR())' only matches
-files of zero size.
+performed with kibibytes, mebibytes, or blocks (of 512 bytes; note: em(not)
+pebibytes) instead. (On some systems additional specifiers are available
+for gibibytes, `tt(g)' or `tt(G)', and tebibytes, `tt(t)' or `tt(T)'.) If a
+size specifier is used, a file is regarded as "exactly" the size if the file
+size rounded up to the next unit is equal to the test size.  Hence
+`tt(*LPAR()Lm1+RPAR())' matches files from 1 byte up to 1 Mebibyte
+inclusive.  Note also that the set of files "less than" the test size only
+includes files that would not match the equality test; hence
+`tt(*LPAR()Lm-1+RPAR())' only matches files of zero size.
 )
 item(tt(^))(
 negates all qualifiers following it
diff --git a/Doc/Zsh/mod_zpty.yo b/Doc/Zsh/mod_zpty.yo
index 3ca031c01..ba59e1783 100644
--- a/Doc/Zsh/mod_zpty.yo
+++ b/Doc/Zsh/mod_zpty.yo
@@ -66,8 +66,8 @@ read matches the var(pattern), even in the non-blocking case.  The return
 status is zero if the string read matches the pattern, or if the command
 has exited but at least one character could still be read.  If the option
 tt(-m) is present, the return status is zero only if the pattern matches.
-As of this writing, a maximum of one megabyte of output can be consumed
-this way; if a full megabyte is read without matching the pattern, the
+As of this writing, a maximum of one mebibyte of output can be consumed
+this way; if a full mebibyte is read without matching the pattern, the
 return status is non-zero.
 
 In all cases, the return status is non-zero if nothing could be read, and
diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index 36c1ae4c2..6c305da34 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -1473,7 +1473,7 @@ is specified with no command.  Defaults to tt(more).
 vindex(REPORTMEMORY)
 item(tt(REPORTMEMORY))(
 If nonnegative, commands whose maximum resident set size (roughly
-speaking, main memory usage) in kilobytes is greater than this
+speaking, main memory usage) in kibibytes is greater than this
 value have timing statistics reported.  The format used to output
 statistics is the value of the tt(TIMEFMT) parameter, which is the same
 as for the tt(REPORTTIME) variable and the tt(time) builtin; note that
@@ -1603,12 +1603,12 @@ sitem(tt(%E))(Elapsed time in seconds.)
 sitem(tt(%P))(The CPU percentage, computed as
 100*(tt(%U)PLUS()tt(%S))/tt(%E).)
 sitem(tt(%W))(Number of times the process was swapped.)
-sitem(tt(%X))(The average amount in (shared) text space used in kilobytes.)
+sitem(tt(%X))(The average amount in (shared) text space used in kibibytes.)
 sitem(tt(%D))(The average amount in (unshared) data/stack space used in
-kilobytes.)
-sitem(tt(%K))(The total space used (tt(%X)PLUS()tt(%D)) in kilobytes.)
+kibibytes.)
+sitem(tt(%K))(The total space used (tt(%X)PLUS()tt(%D)) in kibibytes.)
 sitem(tt(%M))(The  maximum memory the process had in use at any time in
-kilobytes.)
+kibibytes.)
 sitem(tt(%F))(The number of major page faults (page needed to be brought
 from disk).)
 sitem(tt(%R))(The number of minor page faults.)
diff --git a/Etc/BUGS b/Etc/BUGS
index 49e6a5c34..28e4f2b81 100644
--- a/Etc/BUGS
+++ b/Etc/BUGS
@@ -73,3 +73,7 @@ interactive and the subshell is the foreground job.  The USEZLE option is
 always turned off in subshells, for reasons lost to history.  There is a
 related, probably obsolete, vared special case for $TERM set to "emacs".
 ------------------------------------------------------------------------
+47634: missing rlim_t overflow checks in limit/ulimit limit arguments
+Integer values corresponding to the values of RLIM_INFINITY, RLIM_SAVED_MAX,
+RLIM_SAVED_CUR should probably be rejected as well (47639)
+------------------------------------------------------------------------
diff --git a/NEWS b/NEWS
index d05e8b64f..9d218153b 100644
--- a/NEWS
+++ b/NEWS
@@ -11,7 +11,12 @@ The zsh/system module's `zsystem flock` command learnt an -i option to
 set the wait interval used with -t. Additionally, -t now supports
 fractional seconds.
 
+The `limit` builtin accepts more units for resource limits, now shared
+with the `ulimit` builtin allowing limit values to be specified.
+`limit -s somelimit` now reports the shell process' own value of the limit.
 The `limit` and `unlimit` builtins are now available again in csh emulation.
+A few more improvements to the resource limit handling interfaces and
+documentation have been made.
 
 Changes from 5.7.1-test-3 to 5.8
 --------------------------------
diff --git a/Src/Builtins/rlimits.c b/Src/Builtins/rlimits.c
index 5f9c84b0f..5fec90e63 100644
--- a/Src/Builtins/rlimits.c
+++ b/Src/Builtins/rlimits.c
@@ -55,7 +55,8 @@ typedef struct resinfo_T {
  * 1. Add zsh_LIMIT_PRESENT(RLIMIT_XXX) in configure.ac.
  * 2. Add an entry for RLIMIT_XXX to known_resources[].
  *    Make sure the option letter (resinto_T.opt) is unique.
- * 3. Build zsh and run the test B12rlimit.ztst.
+ * 3. Add entry in documentation for the limit and ulimit builtins
+ * 4. Build zsh and run the test B12rlimit.ztst.
  */
 static const resinfo_T known_resources[] = {
     {RLIMIT_CPU, "cputime", ZLIMTYPE_TIME, 1,
@@ -109,12 +110,23 @@ static const resinfo_T known_resources[] = {
 		'i', "pending signals"},
 # endif
 # ifdef HAVE_RLIMIT_MSGQUEUE
-    {RLIMIT_MSGQUEUE, "msgqueue", ZLIMTYPE_NUMBER, 1,
+    {RLIMIT_MSGQUEUE, "msgqueue", ZLIMTYPE_MEMORY, 1,
 		'q', "bytes in POSIX msg queues"},
 # endif
 # ifdef HAVE_RLIMIT_NICE
+    /*
+     * Not max niceness. On Linux ranges [1..40] for that RLIM translates
+     * to [-19..20] meaning that for instance setting it to 20 means
+     * processes can lower their niceness as far down as -1 and with the
+     * default of 0, unprivileged processes can't lower their niceness.
+     *
+     * Increasing niceness is always possible.
+     *
+     * "max nice priority" is the wording used in /proc/self/limits on
+     * Linux.
+     */
     {RLIMIT_NICE, "nice", ZLIMTYPE_NUMBER, 1,
-		'e', "max nice"},
+		'e', "max nice priority"},
 # endif
 # ifdef HAVE_RLIMIT_RTPRIO
     {RLIMIT_RTPRIO, "rt_priority", ZLIMTYPE_NUMBER, 1,
@@ -122,7 +134,7 @@ static const resinfo_T known_resources[] = {
 # endif
 # ifdef HAVE_RLIMIT_RTTIME
     {RLIMIT_RTTIME, "rt_time", ZLIMTYPE_MICROSECONDS, 1,
-		'N', "rt cpu time (microseconds)"},
+		'R', "rt cpu time slice (microseconds)"},
 # endif
     /* BSD */
 # ifdef HAVE_RLIMIT_SBSIZE
@@ -192,9 +204,9 @@ set_resinfo(void)
 	if (!resinfo[i]) {
 	    /* unknown resource */
 	    resinfo_T *info = (resinfo_T *)zshcalloc(sizeof(resinfo_T));
-	    char *buf = (char *)zalloc(12);
+	    char *buf = (char *)zalloc(8 /* UNKNOWN- */ + 3 /* digits */ + 1 /* '\0' */);
 	    snprintf(buf, 12, "UNKNOWN-%d", i);
-	    info->res = - 1;	/* negative value indicates "unknown" */
+	    info->res = -1;	/* negative value indicates "unknown" */
 	    info->name = buf;
 	    info->type = ZLIMTYPE_UNKNOWN;
 	    info->unit = 1;
@@ -255,38 +267,214 @@ printrlim(rlim_t val, const char *unit)
 # endif /* RLIM_T_IS_QUAD_T */
 }
 
+/*
+ * Parse a string into the corresponding value based on the limit type
+ *
+ *   ZLIMTYPE_UNKNOWN / ZLIMTYPE_NUMBER:
+ *     decimal integer without sign only: raw value
+ *
+ *   ZLIMTYPE_TIME / ZLIMTYPE_MICROSECONDS:
+ *     <decimal> only gives seconds or microseconds depending on type
+ *     or:
+ *     [[hour:]min:]sec[.usec]
+ *     or:
+ *     <decimal> with h (hour), m (min), s (sec), ms, us suffix.
+ *
+ *   ZLIMTYPE_MEMORY:
+ *     <decimal> without suffix interpreted as KiB by "limit" (except for
+ *     RLIMIT_MSGQUEUE, see below) and based on resinfo.unit by "ulimit".
+ *
+ *     K/M/G/T/P/E suffix and same with iB suffix use 1024 factor
+ *     KB/MB/GB... use 1000 factor.
+ *
+ *     B for bytes (avoids that mess about default units).
+ *
+ * All suffixes are case insensitive.
+ *
+ * Arguments:
+ *   - s: string to parse
+ *   - lim: resource being limited (from which will derive the type and unit)
+ *   - ulimit: to specify whether we're being called by ulimit or not.
+ *             For ulimit, suffix-less limits are multiplied by the limit's
+ *             unit.
+ *   - err: (return value) error to be returned if any. If a non-NULL value is
+ *           stored there, zstrtorlimt failed and the return value is
+ *           irrelevant (though will be 0).
+ *
+ */
+
 /**/
 static rlim_t
-zstrtorlimt(const char *s, char **t, int base)
+zstrtorlimt(const char *s, int lim, int ulimit, char **err)
 {
     rlim_t ret = 0;
+    const char *orig = s;
+    enum zlimtype type = resinfo[lim]->type;
+    *err = NULL;
 
-    if (strcmp(s, "unlimited") == 0) {
-	if (t)
-	    *t = (char *) s + 9;
+    if (strcmp(s, "unlimited") == 0)
 	return RLIM_INFINITY;
+
+    for (; *s >= '0' && *s <= '9'; s++)
+	ret = ret * 10 + *s - '0';
+
+    if (s == orig) {
+	*err = "decimal integer expected";
+	return 0;
+    }
+
+    if (lim >= RLIM_NLIMITS ||
+	type == ZLIMTYPE_NUMBER ||
+	type == ZLIMTYPE_UNKNOWN) {
+	/*
+	 * pure numeric resource -- only a straight decimal number is
+	 * permitted.
+	 */
+	if (*s) {
+	    *err = "limit must be a decimal integer";
+	    return 0;
+	}
+    }
+    else if (type == ZLIMTYPE_TIME ||
+	     type == ZLIMTYPE_MICROSECONDS) {
+	if (*s) {
+	    int divisor = 1;
+	    int factor = 1;
+	    switch (*s++) {
+
+	    case 'm': /* m for minute or ms for milisecond */
+	    case 'M':
+		if (*s == 's' || *s == 'S') {
+		    s++;
+		    divisor = 1000;
+		}
+		else {
+		    factor = 60;
+		}
+		break;
+
+	    case 'h':
+	    case 'H':
+		factor = 3600;
+		break;
+
+	    case 's':
+	    case 'S':
+		break;
+
+	    case 'u':
+	    case 'U':
+		divisor = 1000000;
+		if (*s == 's' || *s == 'S')
+		    s++;
+		break;
+
+	    case ':':
+		do {
+		    int more = 0;
+		    while (*s >= '0' && *s <= '9') {
+			more = more * 10 + (*s - '0');
+			s++;
+		    }
+		    ret = ret * 60 + more;
+		} while (*s == ':' && ++s);
+		if (*s == '.')
+		    s++;
+		    /* fallthrough */
+		else
+		    break;
+	    case '.':
+		if (type == ZLIMTYPE_MICROSECONDS) {
+		    int frac;
+		    for (frac = 0; *s >= '0' && *s <= '9'; s++) {
+			if (divisor < 1000000) {
+			    /* ignore digits past the 6th */
+			    divisor *= 10;
+			    frac = frac * 10 + (*s - '0');
+			}
+		    }
+		    ret = ret * divisor + frac;
+		}
+		else {
+		    /* fractional part ignored */
+		    while (*s >= '0' && *s <= '9')
+			s++;
+		}
+		break;
+	    default:
+		*err = "invalid time specification";
+		return 0;
+	    }
+
+	    if (*s) {
+		*err = "invalid time specification";
+		return 0;
+	    }
+
+	    ret *= factor;
+	    if (type == ZLIMTYPE_MICROSECONDS)
+		ret *= 1000000 / divisor;
+	    else
+		ret /= divisor;
+	}
+    }
+    else {
+	/*
+	 * memory-type resource
+	 */
+	if (*s) {
+	    if (*s == 'b' || *s == 'B')
+		s++;
+	    else {
+		const char *suffix = "kKmMgGtTpPeE";
+		char *offset;
+
+		if ((offset = strchr(suffix, *s))) {
+		    s++;
+		    if (*s == 'b' || *s == 'B') {
+			/* KB == 1000 */
+			const char *p;
+			for (p = suffix; p <= offset; p += 2)
+			    ret *= 1000;
+			s++;
+		    }
+		    else {
+			/* K/KiB == 1024 */
+			if ((s[0] == 'i' || s[0] == 'I') &&
+			    (s[1] == 'b' || s[1] == 'B'))
+			    s += 2;
+			ret <<= ((offset - suffix) / 2 + 1) * 10;
+		    }
+		}
+	    }
+	    if (*s) {
+		*err = "invalid unit";
+		return 0;
+	    }
+	}
+	else {
+	    if (ulimit)
+		ret *= resinfo[lim]->unit;
+	    else
+#ifdef HAVE_RLIMIT_MSGQUEUE
+		if (lim != RLIMIT_MSGQUEUE)
+		    /*
+		     * Historical quirk. In tcsh's limit (and bash's and mksh's
+		     * ulimit, but not ksh93), that limit expects bytes instead
+		     * of kibibytes and earlier versions of zsh were treating
+		     * it as a ZLIMTYPE_NUMBER.
+		     *
+		     * We still want to treat it as ZLIMTYPE_MEMORY and accept
+		     * KMG... suffixes as it is a number of bytes.
+		     *
+		     * But we carry on taking the value as a number of *bytes*
+		     * in the "limit" builtin for backward compatibility and
+		     * compatibility with tcsh.
+		     */
+#endif
+		    ret *= 1024;
+	}
     }
-# if defined(RLIM_T_IS_QUAD_T) || defined(RLIM_T_IS_LONG_LONG) || defined(RLIM_T_IS_UNSIGNED)
-    if (!base) {
-	if (*s != '0')
-	    base = 10;
-	else if (*++s == 'x' || *s == 'X')
-	    base = 16, s++;
-	else
-	    base = 8;
-    } 
-    if (base <= 10)
-	for (; *s >= '0' && *s < ('0' + base); s++)
-	    ret = ret * base + *s - '0';
-    else
-	for (; idigit(*s) || (*s >= 'a' && *s < ('a' + base - 10))
-	     || (*s >= 'A' && *s < ('A' + base - 10)); s++)
-	    ret = ret * base + (idigit(*s) ? (*s - '0') : (*s & 0x1f) + 9);
-    if (t)
-	*t = (char *)s;
-# else /* !RLIM_T_IS_QUAD_T && !RLIM_T_IS_LONG_LONG && !RLIM_T_IS_UNSIGNED */
-    ret = zstrtol(s, t, base);
-# endif /* !RLIM_T_IS_QUAD_T && !RLIM_T_IS_LONG_LONG && !RLIM_T_IS_UNSIGNED */
     return ret;
 }
 
@@ -317,25 +505,42 @@ showlimitvalue(int lim, rlim_t val)
 	       resinfo[lim]->type == ZLIMTYPE_UNKNOWN)
 	printrlim(val, "\n");	/* pure numeric resource */
     else {
-	/* memory resource -- display with `k' or `M' modifier */
-	if (val >= 1024L * 1024L)
-	    printrlim(val/(1024L * 1024L), "MB\n");
+	/*
+	 * memory resource -- display with KiB/MiB... for exact
+	 * multiples of those units
+	 */
+	const char *units = "KMGTPE";
+	rlim_t v = val;
+	int n = 0;
+	while (units[n] && (v & 1023) == 0 && v >> 10) {
+	    n++;
+	    v >>= 10;
+	}
+	if (n) {
+	    char suffix[] = "XiB\n";
+	    *suffix = units[n-1];
+	    printrlim(v, suffix);
+	}
 	else
-	    printrlim(val/1024L, "kB\n");
+	    printrlim(val, "B\n");
     }
 }
 
-/* Display resource limits.  hard indicates whether `hard' or `soft'  *
- * limits should be displayed.  lim specifies the limit, or may be -1 *
- * to show all.                                                       */
+/*
+ * Display resource limits.  hard indicates whether `hard' or `soft'
+ * limits should be displayed.  lim specifies the limit, or may be -1
+ * to show all.  If `shell' is non-zero, the limits in place for the
+ * shell process retrieved with getrlimits() are shown.
+ */
 
 /**/
 static int
-showlimits(char *nam, int hard, int lim)
+showlimits(char *nam, int hard, int lim, int shell)
 {
     int rt;
+    int ret = 0;
 
-    if (lim >= RLIM_NLIMITS)
+    if (shell || lim >= RLIM_NLIMITS)
     {
 	/*
 	 * Not configured into the shell.  Ask the OS
@@ -357,17 +562,40 @@ showlimits(char *nam, int hard, int lim)
     else
     {
 	/* main loop over resource types */
-	for (rt = 0; rt != RLIM_NLIMITS; rt++)
-	    showlimitvalue(rt, (hard) ? limits[rt].rlim_max :
-			   limits[rt].rlim_cur);
+	for (rt = 0; rt != RLIM_NLIMITS; rt++) {
+	    struct rlimit vals, *plim;
+	    if (shell) {
+		/*
+		 * FIXME: this is dead code as at the moment, there is no API
+		 * for the user can request all limits *of the shell* be
+		 * displayed.
+		 *
+		 * Should we add a "limit -s -a" or "limit -s all"?
+		 */
+		plim = &vals;
+		if (getrlimit(rt, plim) < 0)
+		{
+		    zwarnnam(nam, "can't read \"%s\" limit: %e", resinfo[rt]->name, errno);
+		    ret = 1;
+		    continue;
+		}
+	    }
+	    else
+		plim = &(limits[rt]);
+
+	    showlimitvalue(rt, (hard) ? plim->rlim_max :
+			   plim->rlim_cur);
+	}
     }
 
-    return 0;
+    return ret;
 }
 
-/* Display a resource limit, in ulimit style.  lim specifies which   *
- * limit should be displayed, and hard indicates whether the hard or *
- * soft limit should be displayed.                                   */
+/*
+ * Display a resource limit, in ulimit style.  lim specifies which
+ * limit should be displayed, and hard indicates whether the hard or
+ * soft limit should be displayed.
+ */
 
 /**/
 static int
@@ -394,12 +622,12 @@ printulimit(char *nam, int lim, int hard, int head)
 	if (lim < RLIM_NLIMITS) {
 	    const resinfo_T *info = resinfo[lim];
 	    if (info->opt == 'N')
-		printf("-N %2d: %-29s", lim, info->descr);
+		printf("-N %2d: %-32s ", lim, info->descr);
 	    else
-		printf("-%c: %-32s", info->opt, info->descr);
+		printf("-%c: %-35s ", info->opt, info->descr);
 	}
 	else
-	    printf("-N %2d: %-29s", lim, "");
+	    printf("-N %2d: %-32s ", lim, "");
     }
     /* display the limit */
     if (limit == RLIM_INFINITY)
@@ -511,16 +739,18 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
     rlim_t val;
     int ret = 0;
 
-    hard = OPT_ISSET(ops,'h');
-    if (OPT_ISSET(ops,'s') && !*argv)
+    hard = OPT_ISSET(ops, 'h');
+    if (OPT_ISSET(ops, 's') && !*argv)
 	return setlimits(NULL);
     /* without arguments, display limits */
     if (!*argv)
-	return showlimits(nam, hard, -1);
+	return showlimits(nam, hard, -1, OPT_ISSET(ops, 's'));
     while ((s = *argv++)) {
 	/* Search for the appropriate resource name.  When a name matches (i.e. *
 	 * starts with) the argument, the lim variable changes from -1 to the   *
 	 * number of the resource.  If another match is found, lim goes to -2.  */
+	char *err;
+
 	if (idigit(*s))
 	{
 	    lim = (int)zstrtol(s, NULL, 10);
@@ -543,61 +773,12 @@ bin_limit(char *nam, char **argv, Options ops, UNUSED(int func))
 	}
 	/* without value for limit, display the current limit */
 	if (!(s = *argv++))
-	    return showlimits(nam, hard, lim);
-	if (lim >= RLIM_NLIMITS)
-	{
-	    val = zstrtorlimt(s, &s, 10);
-	    if (*s)
-	    {
-		/* unknown limit, no idea how to scale */
-		zwarnnam(nam, "unknown scaling factor: %s", s);
-		return 1;
-	    }
-	}
-	else if (resinfo[lim]->type == ZLIMTYPE_TIME) {
-	    /* time-type resource -- may be specified as seconds, or minutes or *
-	     * hours with the `m' and `h' modifiers, and `:' may be used to add *
-	     * together more than one of these.  It's easier to understand from *
-	     * the code:                                                        */
-	    val = zstrtorlimt(s, &s, 10);
-	    if (*s) {
-		if ((*s == 'h' || *s == 'H') && !s[1])
-		    val *= 3600L;
-		else if ((*s == 'm' || *s == 'M') && !s[1])
-		    val *= 60L;
-		else if (*s == ':')
-		    val = val * 60 + zstrtorlimt(s + 1, &s, 10);
-		else {
-		    zwarnnam(nam, "unknown scaling factor: %s", s);
-		    return 1;
-		}
-	    }
-	} else if (resinfo[lim]->type == ZLIMTYPE_NUMBER ||
-		   resinfo[lim]->type == ZLIMTYPE_UNKNOWN ||
-		   resinfo[lim]->type == ZLIMTYPE_MICROSECONDS) {
-	    /* pure numeric resource -- only a straight decimal number is
-	    permitted. */
-	    char *t = s;
-	    val = zstrtorlimt(t, &s, 10);
-	    if (s == t) {
-		zwarnnam(nam, "limit must be a number");
-		return 1;
-	    }
-	} else {
-	    /* memory-type resource -- `k', `M' and `G' modifiers are *
-	     * permitted, meaning (respectively) 2^10, 2^20 and 2^30. */
-	    val = zstrtorlimt(s, &s, 10);
-	    if (!*s || ((*s == 'k' || *s == 'K') && !s[1])) {
-		if (val != RLIM_INFINITY)
-		    val *= 1024L;
-	    } else if ((*s == 'M' || *s == 'm') && !s[1])
-		val *= 1024L * 1024;
-	    else if ((*s == 'G' || *s == 'g') && !s[1])
-		val *= 1024L * 1024 * 1024;
-	    else {
-		zwarnnam(nam, "unknown scaling factor: %s", s);
-		return 1;
-	    }
+	    return showlimits(nam, hard, lim, OPT_ISSET(ops, 's'));
+
+	val = zstrtorlimt(s, lim, 0, &err);
+	if (err) {
+	    zwarnnam(nam, "%s: %s", s, err);
+	    return 1;
 	}
 	if (do_limit(nam, lim, val, hard, !hard, OPT_ISSET(ops, 's')))
 	    ret++;
@@ -821,14 +1002,12 @@ bin_ulimit(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		    limit = vals.rlim_max;
 		}
 	    } else {
-		limit = zstrtorlimt(*argv, &eptr, 10);
-		if (*eptr) {
-		    zwarnnam(name, "invalid number: %s", *argv);
+		char *err;
+		limit = zstrtorlimt(*argv, res, 1, &err);
+		if (err) {
+		    zwarnnam(name, "%s: %s", *argv, err);
 		    return 1;
 		}
-		/* scale appropriately */
-		if (res < RLIM_NLIMITS)
-		    limit *= resinfo[res]->unit;
 	    }
 	    if (do_limit(name, res, limit, hard, soft, 1))
 		ret++;
diff --git a/Test/B12limit.ztst b/Test/B12limit.ztst
index 48d33e6e3..c281c523a 100644
--- a/Test/B12limit.ztst
+++ b/Test/B12limit.ztst
@@ -26,3 +26,160 @@ F:report this to zsh-workers mailing list.
   }
 0:check if limit option letters are unique
 
+  if sh -c 'ulimit -f 2048' > /dev/null 2>&1; then
+    (
+      set -o braceccl -o pipefail
+      list=(1b 1{kmgtpe}{,b,ib})
+      for cmd in "limit filesize" ulimit; do
+	for l in $list $list:u; do
+	  $=cmd $l &&
+	    limit filesize &&
+	    ulimit || exit
+	done
+      done | sed 'N;s/\n/ /;s/  */ /g'
+    )
+  else
+    ZTST_skip='Cannot set the filesize limit on this system'
+  fi
+0:filesize suffixes with limit and ulimit
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+>filesize 1B 0
+>filesize 1EiB 2251799813685248
+>filesize 976562500000000KiB 1953125000000000
+>filesize 1EiB 2251799813685248
+>filesize 1GiB 2097152
+>filesize 1000000000B 1953125
+>filesize 1GiB 2097152
+>filesize 1KiB 2
+>filesize 1000B 1
+>filesize 1KiB 2
+>filesize 1MiB 2048
+>filesize 1000000B 1953
+>filesize 1MiB 2048
+>filesize 1PiB 2199023255552
+>filesize 976562500000KiB 1953125000000
+>filesize 1PiB 2199023255552
+>filesize 1TiB 2147483648
+>filesize 976562500KiB 1953125000
+>filesize 1TiB 2147483648
+
+  if sh -c 'ulimit -t 3600' > /dev/null 2>&1; then
+  (
+    set -o pipefail
+    list=(1h 30m 20s 30 1:23:45.123456 2:23 56.4)
+    for cmd in "limit cputime" "ulimit -t"; do
+      for l in $list ${(MU)list:#*[a-z]*}; do
+        $=cmd $l &&
+	  limit cputime &&
+	  ulimit -t || exit
+      done
+    done | sed 'N;s/\n/ /;s/  */ /g'
+  )
+  else
+    ZTST_skip='Cannot set the cputime limit on this system'
+  fi
+0:time limit formats
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 0:00:30 30
+>cputime 1:23:45 5025
+>cputime 0:02:23 143
+>cputime 0:00:56 56
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+>cputime 0:00:30 30
+>cputime 1:23:45 5025
+>cputime 0:02:23 143
+>cputime 0:00:56 56
+>cputime 1:00:00 3600
+>cputime 0:30:00 1800
+>cputime 0:00:20 20
+
+  ulimit 1Kite
+  ulimit 1D
+  ulimit 1s
+  ulimit 1MBA
+  limit cputime 1k
+  limit cputime 1:0s
+  limit cputime 1ss
+  limit cputime 1msx
+  limit cputime 1.0s
+  limit cputime .1
+  limit descriptors 1k
+  limit descriptors 1h
+  limit descriptors 1:0
+1:invalid limit input
+?(eval):ulimit:1: 1Kite: invalid unit
+?(eval):ulimit:2: 1D: invalid unit
+?(eval):ulimit:3: 1s: invalid unit
+?(eval):ulimit:4: 1MBA: invalid unit
+?(eval):limit:5: 1k: invalid time specification
+?(eval):limit:6: 1:0s: invalid time specification
+?(eval):limit:7: 1ss: invalid time specification
+?(eval):limit:8: 1msx: invalid time specification
+?(eval):limit:9: 1.0s: invalid time specification
+?(eval):limit:10: .1: decimal integer expected
+?(eval):limit:11: 1k: limit must be a decimal integer
+?(eval):limit:12: 1h: limit must be a decimal integer
+?(eval):limit:13: 1:0: limit must be a decimal integer
-- 
2.25.1





^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2020-11-27 20:13       ` Stephane Chazelas
@ 2020-11-27 20:36         ` Daniel Shahaf
  2020-11-28  6:52           ` zsh coding style (was about a limit patch review) Stephane Chazelas
  2020-11-28  8:16         ` [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
  1 sibling, 1 reply; 30+ messages in thread
From: Daniel Shahaf @ 2020-11-27 20:36 UTC (permalink / raw)
  To: zsh-workers

Stephane Chazelas wrote on Fri, 27 Nov 2020 20:13 +00:00:
> 2020-11-27 16:39:49 +0000, Daniel Shahaf:
> [...]
> > Please specify in the docstring the types, values, and meanings of the
> > formal parameters.
> 
> I've tried to address that in this v3 patch. Specifying the type
> (as in char*, int...) seems redundant though and not generally
> done in other functions, so I've left it out for now.

Of course, DRY; but still, notice how one of the ints is actually
a boolean, so the formal type information doesn't convey everything.

> +++ b/Src/Builtins/rlimits.c
> @@ -255,38 +267,214 @@ printrlim(rlim_t val, const char *unit)
> +/*
> + * Parse a string into the corresponding value based on the limit type
> + *
> + *   ZLIMTYPE_UNKNOWN / ZLIMTYPE_NUMBER:
> + *     decimal integer without sign only: raw value
> + *
> + *   ZLIMTYPE_TIME / ZLIMTYPE_MICROSECONDS:
> + *     <decimal> only gives seconds or microseconds depending on type
> + *     or:
> + *     [[hour:]min:]sec[.usec]
> + *     or:
> + *     <decimal> with h (hour), m (min), s (sec), ms, us suffix.
> + *
> + *   ZLIMTYPE_MEMORY:
> + *     <decimal> without suffix interpreted as KiB by "limit" (except for
> + *     RLIMIT_MSGQUEUE, see below) and based on resinfo.unit by "ulimit".
> + *
> + *     K/M/G/T/P/E suffix and same with iB suffix use 1024 factor
> + *     KB/MB/GB... use 1000 factor.
> + *
> + *     B for bytes (avoids that mess about default units).
> + *
> + * All suffixes are case insensitive.
> + *
> + * Arguments:
> + *   - s: string to parse
> + *   - lim: resource being limited (from which will derive the type and unit)
> + *   - ulimit: to specify whether we're being called by ulimit or not.
> + *             For ulimit, suffix-less limits are multiplied by the limit's
> + *             unit.

Suggest to add the word "boolean" to the description of this parameter.

Since you've already gone and described it in prose, I'd consider to
also rename it to something like
«multiply_suffixless_limits_by_implied_unit_p» (trailing _p for
"predicate", i.e., signifies the variable is a boolean), to avoid caller–callee coupling.

> + *   - err: (return value) error to be returned if any. If a non-NULL value is
> + *           stored there, zstrtorlimt failed and the return value is
> + *           irrelevant (though will be 0).

You don't actually say anywhere what the return value will be when there
_isn't_ an error.

Cheers,

Daniel

P.S.  As I said elsethread, feel free to stick this patch on a short-
      lived topic branch, if that's easier to you than posting revisions
      to a mailing list.

> + */
> +
>  /**/
>  static rlim_t
> -zstrtorlimt(const char *s, char **t, int base)
> +zstrtorlimt(const char *s, int lim, int ulimit, char **err)
>  {



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
  2020-11-27 18:24         ` Jun. T
  2020-11-27 18:34           ` Daniel Shahaf
@ 2020-11-27 20:46           ` Stephane Chazelas
  1 sibling, 0 replies; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-27 20:46 UTC (permalink / raw)
  To: Jun. T; +Cc: zsh-workers

2020-11-28 03:24:04 +0900, Jun. T:
[...]
> I guess you mean we should reject RLIM_INFINITY, and yes I agree with it.
> How about the patch below (to your v2)?
[...]

It should probably reject RLIM_SAVED_MAX and RLIM_SAVED_CUR like
yash does as well.

[...]
> +	if ((tmp = ret*unit) < ret) {
> +	    *err = "limit out of range";
> +	    return 0;
[...]

I don't think that's a valid check. It would still accept 17EiB
for instance which is the same as 1EiB with rlimt_t === uint64
and greater than 17.

The problem may be better addressed by reviewing overflow
handling everywhere in the code, probably factorising all code
that does parses decimal integer representations, maybe using
strtoll(), etc. I remember seeing a few bug reports here about
overflow handling in arithmetic evaluation, I'm not sure they've
all been fixed. And I'd be surprised if there aren't other
places where this kind of string to number manual handling
without overflow check is done. 

It probably calls for somebody having a thorough look at the
code, and come up with a plan to address the issue globally,
which goes beyond the scope of this rlimit improvement, I say.

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* zsh coding style (was about a limit patch review)
  2020-11-27 20:36         ` Daniel Shahaf
@ 2020-11-28  6:52           ` Stephane Chazelas
  2020-12-01 16:47             ` Daniel Shahaf
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-28  6:52 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

2020-11-27 20:36:40 +0000, Daniel Shahaf:
[...]
> Of course, DRY; but still, notice how one of the ints is actually
> a boolean, so the formal type information doesn't convey everything.
[...]
> > + * Arguments:
> > + *   - s: string to parse
> > + *   - lim: resource being limited (from which will derive the type and unit)
> > + *   - ulimit: to specify whether we're being called by ulimit or not.
> > + *             For ulimit, suffix-less limits are multiplied by the limit's
> > + *             unit.
> 
> Suggest to add the word "boolean" to the description of this parameter.
> 
> Since you've already gone and described it in prose, I'd consider to
> also rename it to something like
> «multiply_suffixless_limits_by_implied_unit_p» (trailing _p for
> "predicate", i.e., signifies the variable is a boolean), to avoid caller–callee coupling.
> 
> > + *   - err: (return value) error to be returned if any. If a non-NULL value is
> > + *           stored there, zstrtorlimt failed and the return value is
> > + *           irrelevant (though will be 0).
> 
> You don't actually say anywhere what the return value will be when there
> _isn't_ an error.
[...]

Believe it or not, I had "(boolean)" and a "Returns:..."
initially, and took them out before submitting as they would
have been out of character among the other functions in that
file (and the rest of the code from a cursory look) which are
rather minimalist and avoid stating the obvious.

This function description is already by far the least dry and
probably the most RY in that file. To the point that I'm
wondering if you're pulling my leg for being overly wordy here.

About the _p suffix, the only usage of it in the code (beside
has_p) I could find for it was 
save_params(Estate state, Wordcode pc, LinkList *restore_p, LinkList *remove_p)
in exec.c, but that "p" looks like it's for "pointer" (as in
return value, though could also be "param"), not "predicate".
I've never heard of _p for predicate before, but then again I've
not done much C development lately.

I'm all for having a coding style. Even a template for function
header could be useful. That would probably help make the
code more legible (though in general, I do find zsh's code
pretty legible, orders of magnitude more so than ksh93's for
instance)

I did read Etc/zsh-development-guide before submitting my first
patch. The relevant bit there is:

} * Function declarations must look like this:
} 
}   /**/
}   int
}   foo(char *s, char **p)
}   {
}       function body
}   }
} 
}   There must be an empty line, a line with "/**/", a line with the
}   type of the function, and finally the name of the function with typed
}   arguments.  These lines must not be indented.  The script generating
}   function prototypes and the ansi2knr program depend on this format.

Maybe that document could be updated to say more precisely how
new code should look like if we're to enforce a new coding
style.

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2020-11-27 20:13       ` Stephane Chazelas
  2020-11-27 20:36         ` Daniel Shahaf
@ 2020-11-28  8:16         ` Stephane Chazelas
  2021-03-27 21:21           ` Lawrence Velázquez
  1 sibling, 1 reply; 30+ messages in thread
From: Stephane Chazelas @ 2020-11-28  8:16 UTC (permalink / raw)
  To: Daniel Shahaf, Zsh hackers list

[sorry, should have been v3 in the subject of the email I'm
replying to by the way).

2020-11-27 20:13:42 +0000, Stephane Chazelas:
[...]
> I've tried to address that in this v3 patch. Specifying the type
> (as in char*, int...) seems redundant though and not generally
> done in other functions, so I've left it out for now.
[...]
> +	    if (shell) {
> +		/*
> +		 * FIXME: this is dead code as at the moment, there is no API
> +		 * for the user can request all limits *of the shell* be
> +		 * displayed.
> +		 *
> +		 * Should we add a "limit -s -a" or "limit -s all"?
> +		 */
[...]

And in my initial email:

> - 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.

Maybe one approach could be to introduce some $shell_limits and
$children_limits special associative arrays indexed on limit
name (or number for those limits that don't have a name yet).

Which would allow things like:

(( shell_limits[filesize] >= min )) || die "can't do"

There's the problem of RLIM_INFINITY and the fact that rlim_t is
unsigned so if 64bit, allows values beyond the integer ones
supported by arithmetic evaluation.

Both could be addressed by making the value a floating point
number (append a . or e0 and use inf for RLIM_INFINITY), but
that sounds less than ideal as we're losing precision and limits
are really integers in the first place (and for consistency
limit/ulimit would need to be updated to accept floating point
on input).

There's also the problem that "limit" (contrary to ulimit) lets
you set more than one limit at a time, but not print more than
one limit at a time ("limit cputime" prints the value of
"cputime" but "limit cputime filesize" tries to set the cputime
limit to "filesize").

So, maybe a better approach is to add a "-p" for "print":

- limit or limit -p prints all the known limits for children
- limit -ps prints all the known limits for the shell
- limit -p filesize or limit filesize prints the filesize limit
- limit -p filesize cputime prints the filesize and cputime
  limits.

And maybe a "-r" for "raw" though there's always the problem of
RLIM_INFINITY and uint64.

And of course there's always the possibility of ignoring the
problem altogether as it's not as if anybody is going to miss
that feature very badly.

At the moment, one can always do:

(( ${$(ulimit -f)//unlimited/inf} >= ceil(min / 512.) )) ||
  die "can't do"

(though that's children_limits, and bearing in mind that ulimit
rounds down the number so we'll get false positives like when
min is 1 and the limit is 511B, rounded down to 0).

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: zsh coding style (was about a limit patch review)
  2020-11-28  6:52           ` zsh coding style (was about a limit patch review) Stephane Chazelas
@ 2020-12-01 16:47             ` Daniel Shahaf
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Shahaf @ 2020-12-01 16:47 UTC (permalink / raw)
  To: zsh-workers

Stephane Chazelas wrote on Sat, Nov 28, 2020 at 06:52:01 +0000:
> Believe it or not, I had "(boolean)" and a "Returns:..."
> initially, and took them out before submitting as they would
> have been out of character among the other functions in that
> file (and the rest of the code from a cursory look) which are
> rather minimalist and avoid stating the obvious.

Every docstring should state the permitted values of each parameter and the
possible return values.  Yes, some existing functions fall short of this
standard, but there's no reason not to hold _new_ functions to it.  That
doesn't pay off existing technical debt, but it avoids accruing further debt.

> This function description is already by far the least dry and
> probably the most RY in that file. To the point that I'm
> wondering if you're pulling my leg for being overly wordy here.

No, I'm not.

> About the _p suffix, the only usage of it in the code (beside
> has_p) I could find for it was 
> save_params(Estate state, Wordcode pc, LinkList *restore_p, LinkList *remove_p)
> in exec.c, but that "p" looks like it's for "pointer" (as in
> return value, though could also be "param"), not "predicate".
> I've never heard of _p for predicate before, but then again I've
> not done much C development lately.
> 

I hear it's more common in lisp circles.  It's a form of Hungarian notation.

> I'm all for having a coding style. Even a template for function
> header could be useful. That would probably help make the
> code more legible (though in general, I do find zsh's code
> pretty legible, orders of magnitude more so than ksh93's for
> instance)
> 
> I did read Etc/zsh-development-guide before submitting my first
> patch. The relevant bit there is:
> 
> } * Function declarations must look like this:
> } 
> }   /**/
> }   int
> }   foo(char *s, char **p)
> }   {
> }       function body
> }   }
> } 
> }   There must be an empty line, a line with "/**/", a line with the
> }   type of the function, and finally the name of the function with typed
> }   arguments.  These lines must not be indented.  The script generating
> }   function prototypes and the ansi2knr program depend on this format.
> 
> Maybe that document could be updated to say more precisely how
> new code should look like if we're to enforce a new coding
> style.

I wasn't trying to enforce a new coding style.  I was simply reviewing the
docstring to the same standard I've reviewed every other docstring I've ever
reviewed, either on this list or elsewhere.

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2020-11-28  8:16         ` [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
@ 2021-03-27 21:21           ` Lawrence Velázquez
  2021-03-31 18:06             ` Stephane Chazelas
  0 siblings, 1 reply; 30+ messages in thread
From: Lawrence Velázquez @ 2021-03-27 21:21 UTC (permalink / raw)
  To: zsh-workers; +Cc: Stephane Chazelas

On Sat, Nov 28, 2020, at 3:16 AM, Stephane Chazelas wrote:
> [sorry, should have been v3 in the subject of the email I'm
> replying to by the way).
> 
> 2020-11-27 20:13:42 +0000, Stephane Chazelas:
> [...]
> > I've tried to address that in this v3 patch. Specifying the type
> > (as in char*, int...) seems redundant though and not generally
> > done in other functions, so I've left it out for now.

Any further feedback on this?

vq


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2020-11-27 12:19       ` Oliver Kiddle
@ 2021-03-27 21:25         ` Lawrence Velázquez
  2021-04-03 14:57           ` Lawrence Velázquez
  0 siblings, 1 reply; 30+ messages in thread
From: Lawrence Velázquez @ 2021-03-27 21:25 UTC (permalink / raw)
  To: zsh-workers

Anything else on this? (Sorry if I end up sending too many messages about these ulimit changes; it's a little difficult to follow the threads.)

vq


On Fri, Nov 27, 2020, at 7:19 AM, Oliver Kiddle wrote:
> Stephane Chazelas wrote:
> > By the way, should I cache that list of options retrieved from
> > the output of "ulimit -a"? After all, it's not going to change
> 
> I wouldn't bother. It's a shell builtin, everything it outputs is
> straight from memory so the only cost is a single fork. No delay is
> discernable.
> 
> > from one run to the next. But then again, a $(ulimit -a) command
> > substitution is going to be fast and caching uses up memory. Do
> > you have recommendations as to when to cache and when not to?
> 
> Mostly it comes down to what seems sensible in a particular case. The
> main criteria being if a delay is noticable. But it would also depend
> whether it is the type of command that is likely used repeatedly in a
> session (which ulimit probably isn't much). There's also the choice
> of just cache in a variable in memory vs. using the cache functions
> to store it in a file. I'm more inclined towards that when it is very
> large.
> 
> Oliver
> 
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest)
  2021-03-27 21:21           ` Lawrence Velázquez
@ 2021-03-31 18:06             ` Stephane Chazelas
  0 siblings, 0 replies; 30+ messages in thread
From: Stephane Chazelas @ 2021-03-31 18:06 UTC (permalink / raw)
  To: Lawrence Velázquez; +Cc: zsh-workers

2021-03-27 17:21:14 -0400, Lawrence Velázquez:
> On Sat, Nov 28, 2020, at 3:16 AM, Stephane Chazelas wrote:
> > [sorry, should have been v3 in the subject of the email I'm
> > replying to by the way).
> > 
> > 2020-11-27 20:13:42 +0000, Stephane Chazelas:
> > [...]
> > > I've tried to address that in this v3 patch. Specifying the type
> > > (as in char*, int...) seems redundant though and not generally
> > > done in other functions, so I've left it out for now.
> 
> Any further feedback on this?
[...]

IIRC, the patchset was awaiting review by Daniel Shahaf. Daniel
had requested that it be further split, but I stand by my
original position that it would be impractical as all changes
are intertwined, and not worth the effort.

I'll try and rebase them to the current git head and send a v4.

-- 
Stephane



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2021-03-27 21:25         ` Lawrence Velázquez
@ 2021-04-03 14:57           ` Lawrence Velázquez
  2021-04-10 20:11             ` Lawrence Velázquez
  0 siblings, 1 reply; 30+ messages in thread
From: Lawrence Velázquez @ 2021-04-03 14:57 UTC (permalink / raw)
  To: zsh-workers

On Sat, Mar 27, 2021, at 5:25 PM, Lawrence Velázquez wrote:
> Anything else on this?

ping

vq


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2021-04-03 14:57           ` Lawrence Velázquez
@ 2021-04-10 20:11             ` Lawrence Velázquez
  0 siblings, 0 replies; 30+ messages in thread
From: Lawrence Velázquez @ 2021-04-10 20:11 UTC (permalink / raw)
  To: zsh-workers

On Sat, Apr 3, 2021, at 10:57 AM, Lawrence Velázquez wrote:
> On Sat, Mar 27, 2021, at 5:25 PM, Lawrence Velázquez wrote:
> > Anything else on this?
> 
> ping

ping mk II

vq


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] ulimit option completions using ulimit -a output
  2020-11-26 20:14   ` [PATCH] ulimit option completions using ulimit -a output Stephane Chazelas
  2020-11-27  7:13     ` Stephane Chazelas
@ 2021-04-13 14:35     ` Daniel Shahaf
  1 sibling, 0 replies; 30+ messages in thread
From: Daniel Shahaf @ 2021-04-13 14:35 UTC (permalink / raw)
  To: Stephane Chazelas; +Cc: Zsh hackers list

Stephane Chazelas wrote on Thu, Nov 26, 2020 at 20:14:31 +0000:
> 2020-11-26 00:43:33 +0100, Oliver Kiddle:
> [...]
> > >   * 1. Add zsh_LIMIT_PRESENT(RLIMIT_XXX) in configure.ac.
> > >   * 2. Add an entry for RLIMIT_XXX to known_resources[].
> > >   *    Make sure the option letter (resinto_T.opt) is unique.
> > > - * 3. Build zsh and run the test B12rlimit.ztst.
> > > + * 3. Add entry in documentation for the limit and ulimit builtins
> > > + * 4. Build zsh and run the test B12rlimit.ztst.
> > 
> > 5. Update Completion/Zsh/Command/_ulimit
> > 
> > Units were often fairly well documented there but are missing in a
> > couple of cases so you may be able to add a couple.
> [...]
> 
> The problem is that the list of ulimit options is system
> dependent, so I propose the improvement below which uses the
> output of ulimit -a to build the list of completions.

Sorry for the very large delay on getting to review this.  Thanks
Lawrence for fishing it out of the archives.  Review:

> From: Stephane Chazelas <stephane@chazelas.org>
> Date: Thu, 26 Nov 2020 19:22:03 +0000
> Subject: [PATCH] ulimit option completions using ulimit -a output
> 
> The list of options supported by ulimit is system dependent. Rather than
> hardcoding the full list of options supported on any system in the
> completer, we get the list of options from the output of "ulimit -a".

+1

> That means less descriptive descriptions but more relevant completions
> to the user (and not having to update the completer every time a new
> limit is added).

Should we copy the more-descriptive descriptions to the output of
`ulimit -a`?  (I.e., known_resources[] in Src/Modules/rlimits.c)

> +++ b/Completion/Zsh/Command/_ulimit
> @@ -2,18 +2,16 @@
>  
>  [[ $PREFIX = u* ]] && compadd unlimited && return 0
>  
> +local -a opts
> +setopt localoptions extendedglob
> +
> +opts=(
> +  ${${${(fo)"$(ulimit -a)"}:#-N*}%%  *}
> +)
>  _arguments -s \
>    '-H[set hard limits]' \
>    '-S[set soft and hard limits (with -H)]' \
> -  '(-H -S -c -d -f -l -m -n -s -t *)-a[list all current resource limits]' \
> -  '-c[core dump size limit]:max core dump size (512-byte blocks)' \
> -  '-d[maximum size of data segment]:maximum size of data segment (K-bytes)' \
> -  '-f[size of largest file allowed]:size of largest file allowed (512-byte blocks)' \
> -  '-l[maximum size of locked in memory]:maximum size of locked in memory (K-bytes)' \
> -  '-m[maximum size of physical memory]:maximum size of physical memory (K-bytes)' \
> -  '-n[maximum no. of open file descriptors]:maximum no. of open file descriptors' \
> -  '-s[stack size limit]:stack size limit (K-bytes)' \
> -  '-t[maximum cpu time per process]:maximum cpu time per process (seconds)' \
> -  '-u[processes available to the user]:processes' \
> -  '-v[maximum size of virtual memory]:maximum size of virtual memory (K-bytes)' \
> -  '*:size of largest file allowed'
> +  '-N+[specify limit by number]:limit number::limit value' \

[Corrected per 47661]

Why is the second argument optional?  s/::/: :/?

Completing «-N 15 <TAB>» doesn't offer "rt cpu time (microseconds)",
even though the output of `ulimit -a` does show that description.

Completing after «-N <TAB>» could offer "15".

> +  "(-N ${(M@j: :)opts#-?} *)-a[list all current resource limits]" \
> +  ${opts/(#b)(-?): (*)/$match[1][$match[2]]:$match[2]} \

Escape colons in ${match[1]} and ${match[2]}.

It would be nice to retain the current values in in $opts and then
include them in the `message' part of the spec.

> +  '*:file size (blocks)'

Sorry, what's this?  The manual does mention additional positional
arguments, but doesn't describe them, does it?

Sorry again for our very late review.

Review points that aren't regressions aren't blockers.

Daniel


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2021-04-13 14:36 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 21:49 [PATCHv1] [long] improvements to limit/ulimit API and doc Stephane Chazelas
2020-11-25  0:35 ` Daniel Shahaf
2020-11-25  6:44   ` Stephane Chazelas
2020-11-27 17:16     ` Daniel Shahaf
2020-11-26  6:57   ` [PATCHv2 1/2] [long] improvements to limit/ulimit API and doc ((un)limit in csh emulation) Stephane Chazelas
2020-11-25 23:43 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Oliver Kiddle
2020-11-26 20:14   ` [PATCH] ulimit option completions using ulimit -a output Stephane Chazelas
2020-11-27  7:13     ` Stephane Chazelas
2020-11-27  8:15       ` Felipe Contreras
2020-11-27 12:19       ` Oliver Kiddle
2021-03-27 21:25         ` Lawrence Velázquez
2021-04-03 14:57           ` Lawrence Velázquez
2021-04-10 20:11             ` Lawrence Velázquez
2021-04-13 14:35     ` Daniel Shahaf
2020-11-26 20:58   ` [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
2020-11-27 16:39     ` Daniel Shahaf
2020-11-27 20:13       ` Stephane Chazelas
2020-11-27 20:36         ` Daniel Shahaf
2020-11-28  6:52           ` zsh coding style (was about a limit patch review) Stephane Chazelas
2020-12-01 16:47             ` Daniel Shahaf
2020-11-28  8:16         ` [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
2021-03-27 21:21           ` Lawrence Velázquez
2021-03-31 18:06             ` Stephane Chazelas
2020-11-26 11:19 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Jun T
2020-11-26 13:55   ` Stephane Chazelas
2020-11-26 15:22     ` Jun. T
2020-11-26 17:23       ` Stephane Chazelas
2020-11-27 18:24         ` Jun. T
2020-11-27 18:34           ` Daniel Shahaf
2020-11-27 20:46           ` Stephane Chazelas

zsh-workers

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/zsh-workers

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 zsh-workers zsh-workers/ http://inbox.vuxu.org/zsh-workers \
		zsh-workers@zsh.org
	public-inbox-index zsh-workers

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/zsh/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git