Re: [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest)

[Stephane Chazelas <stephane@chazelas.org>]

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