'typeset -F' behaves unexpectedly for "-F 1" and "-F 0"

'typeset -F' behaves unexpectedly for "-F 1" and "-F 0"

[David Hughes]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

If I run "typeset -F 1 pi=3.14159", I receive a warning of an invalid base,
and the parameter is not initialised. However, the parameter appears to be
created, and thereafter has the expected rounding attribute:

$ typeset -F 1 pi=3.14159
typeset: invalid base (must be 2 to 36 inclusive): 1
$ echo $pi
0.0
$ pi=2.71828
$ echo $pi
2.7

With "-F 0", the warning is also shown, and the parameter is also
configured as a floating point, but when I expand the parameter its
precision is set to the default 10 decimal places:

$ unset pi
$ typeset -F 0 pi=3.14159
typeset: invalid base (must be 2 to 36 inclusive): 0
$ echo $pi
0.0000000000
$ pi=2
$ echo $pi
2.0000000000

The behaviour I expect is that 'pi' should be rounded to one decimal place
with "-F 1", and to the nearest integer with "-F 0".  As a fabricated
example:

$ typeset -F 1 pi=3.14159
$ echo $pi
3.1
$ typeset -F 0 e=2.71828
$ echo $e
3

For values of "-F" greater than one, zsh behaves as I expect: no warnings
are issued, and the expanded value is rounded to the specified number of
places:

$ typeset -F 3 pi=3.14159
$ echo $pi
3.142

I have replicated this in zsh 5.0.5, and I do not see any related bug when
searching http://sourceforge.net/p/zsh/bugs/ for "typeset" or "float", so I
guess this issue is still extant in the dev version.

I didn't have enough C skill to understand the relevant parameter code and
structs from bin_typeset() down to typeset_setbase() within an hour, but
please let me know if I can provide more information at a simpler level.

PATCH: Fix two bugs in typeset_setbase

[Mikael Magnusson]

We refused to set the base outside [2,36], but this restriction should
only apply to integers (where the base is actually the base). For float
values, the precision can be anything. We also failed to actually enforce
this limitation in either of these cases, but only printed the error
message. We now only update the base if it was valid.

---
 Src/builtin.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Src/builtin.c b/Src/builtin.c
index 0113757..1243263 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1865,7 +1865,7 @@ typeset_setbase(const char *name, Param pm, Options ops, int on, int always)
 
     if (arg) {
 	char *eptr;
-	pm->base = (int)zstrtol(arg, &eptr, 10);
+	int base = (int)zstrtol(arg, &eptr, 10);
 	if (*eptr) {
 	    if (on & PM_INTEGER)
 		zwarnnam(name, "bad base value: %s", arg);
@@ -1873,11 +1873,12 @@ typeset_setbase(const char *name, Param pm, Options ops, int on, int always)
 		zwarnnam(name, "bad precision value: %s", arg);
 	    return 1;
 	}
-	if (pm->base < 2 || pm->base > 36) {
+	if ((on & PM_INTEGER) && (base < 2 || base > 36)) {
 	    zwarnnam(name, "invalid base (must be 2 to 36 inclusive): %d",
-		     pm->base);
+		     base);
 	    return 1;
 	}
+	pm->base = base;
     } else if (always)
 	pm->base = 0;
 
-- 
2.4.0

Re: PATCH: Fix two bugs in typeset_setbase

[Mikael Magnusson]

On Sun, May 3, 2015 at 10:36 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> We refused to set the base outside [2,36], but this restriction should
> only apply to integers (where the base is actually the base). For float
> values, the precision can be anything. We also failed to actually enforce
> this limitation in either of these cases, but only printed the error
> message. We now only update the base if it was valid.

Perhaps we should apply some limit to the precision of floats? For example,
typeset -F 100000000 foo; echo $foo
succeeds, and at least in my setup, causes typing at the next
commandline to be very slow, because of multiple calls to
setunderscore(). It doesn't seem to affect zsh -f. This could also be
a case of "don't do that then". :)

-- 
Mikael Magnusson

Re: PATCH: Fix two bugs in typeset_setbase

[Peter Stephenson]

On Sun, 3 May 2015 22:43:46 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> Perhaps we should apply some limit to the precision of floats? For example,
> typeset -F 100000000 foo; echo $foo
> succeeds, and at least in my setup, causes typing at the next
> commandline to be very slow, because of multiple calls to
> setunderscore(). It doesn't seem to affect zsh -f. This could also be
> a case of "don't do that then". :)

Yes, it's not exactly a bug... but I guess it's easy to set it to some
documented ceiling where it's definitely not going to make a practical
difference.  100 or 1000?

pws

Re: PATCH: Fix two bugs in typeset_setbase

[Mikael Magnusson]

On Sun, May 3, 2015 at 11:24 PM, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> On Sun, 3 May 2015 22:43:46 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> Perhaps we should apply some limit to the precision of floats? For example,
>> typeset -F 100000000 foo; echo $foo
>> succeeds, and at least in my setup, causes typing at the next
>> commandline to be very slow, because of multiple calls to
>> setunderscore(). It doesn't seem to affect zsh -f. This could also be
>> a case of "don't do that then". :)
>
> Yes, it's not exactly a bug... but I guess it's easy to set it to some
> documented ceiling where it's definitely not going to make a practical
> difference.  100 or 1000?

Came across this comment while poking around earlier (in
params.c:convfloat()), I guess 1000 should be a very safe limit.

    /*
     * The difficulty with the buffer size is that a %f conversion
     * prints all digits before the decimal point: with 64 bit doubles,
     * that's around 310.  We can't check without doing some quite
     * serious floating point operations we'd like to avoid.
     * Then we are liable to get all the digits
     * we asked for after the decimal point, or we should at least
     * bargain for it.  So we just allocate 512 + digits.  This
     * should work until somebody decides on 128-bit doubles.
     */


-- 
Mikael Magnusson

Re: PATCH: Fix two bugs in typeset_setbase

[Mikael Magnusson]

On Mon, May 4, 2015 at 12:40 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On Sun, May 3, 2015 at 11:24 PM, Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
>> On Sun, 3 May 2015 22:43:46 +0200
>> Mikael Magnusson <mikachu@gmail.com> wrote:
>>> Perhaps we should apply some limit to the precision of floats? For example,
>>> typeset -F 100000000 foo; echo $foo
>>> succeeds, and at least in my setup, causes typing at the next
>>> commandline to be very slow, because of multiple calls to
>>> setunderscore(). It doesn't seem to affect zsh -f. This could also be
>>> a case of "don't do that then". :)
>>
>> Yes, it's not exactly a bug... but I guess it's easy to set it to some
>> documented ceiling where it's definitely not going to make a practical
>> difference.  100 or 1000?
>
> Came across this comment while poking around earlier (in
> params.c:convfloat()), I guess 1000 should be a very safe limit.
>
>     /*
>      * The difficulty with the buffer size is that a %f conversion
>      * prints all digits before the decimal point: with 64 bit doubles,
>      * that's around 310.  We can't check without doing some quite
>      * serious floating point operations we'd like to avoid.
>      * Then we are liable to get all the digits
>      * we asked for after the decimal point, or we should at least
>      * bargain for it.  So we just allocate 512 + digits.  This
>      * should work until somebody decides on 128-bit doubles.
>      */

Oops, the 310 text speaks of the digits before the decimal point (not
sure how I missed it, it's pretty explicit about it). I guess this
means the practical limit is different for -E and -F. I don't know
enough details for floats to make a reasonable suggestion (but
probably doing calculations with multiple thousand digits of precision
is not a job for the shell). I've committed my patch and if someone
wants to plop in a limit, be my guest (or tell me what to make it).

-- 
Mikael Magnusson