* PATCH: hist: remove wrong NULL terminator @ 2015-01-09 12:43 Mikael Magnusson 2015-01-09 17:45 ` Ray Andrews 2015-01-10 7:08 ` PATCH: hist: remove wrong NULL terminator Bart Schaefer 0 siblings, 2 replies; 36+ messages in thread From: Mikael Magnusson @ 2015-01-09 12:43 UTC (permalink / raw) To: zsh-workers This actually writes a NULL to some arbitrary location in the caller function's stack. Found by Coverity (Issue 1255746). The start of the quote() function does char **str = tr; and is called like this, quote(&sline); sline in turn is just a char *sline; The result of str[1] = NULL; is then, as far as I can tell, not anything good. I also can't see any other thing that might have been intended to be NULL-terminated here, so just remove it. --- Src/hist.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Src/hist.c b/Src/hist.c index e65d78b..3dc0472 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -2179,7 +2179,6 @@ quote(char **tr) *rptr++ = *ptr; *rptr++ = '\''; *rptr++ = 0; - str[1] = NULL; return 0; } -- 2.2.0.GIT ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 12:43 PATCH: hist: remove wrong NULL terminator Mikael Magnusson @ 2015-01-09 17:45 ` Ray Andrews 2015-01-09 18:33 ` Lawrence Velázquez 2015-01-09 19:38 ` Mikael Magnusson 2015-01-10 7:08 ` PATCH: hist: remove wrong NULL terminator Bart Schaefer 1 sibling, 2 replies; 36+ messages in thread From: Ray Andrews @ 2015-01-09 17:45 UTC (permalink / raw) To: zsh-workers On 01/09/2015 04:43 AM, Mikael Magnusson wrote: > This actually writes a NULL to some arbitrary location in the caller function's stack. Found by Coverity (Issue 1255746). > > The start of the quote() function does char **str = tr; and is called like this, > quote(&sline); > sline in turn is just a char *sline; > The result of str[1] = NULL; is then, as far as I can tell, not anything good. I also can't see any other thing that might have been intended to be NULL-terminated here, so just remove it. Holy Cow. That's just been sitting there for god knows how long? ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 17:45 ` Ray Andrews @ 2015-01-09 18:33 ` Lawrence Velázquez 2015-01-09 18:36 ` Lawrence Velázquez 2015-01-09 19:38 ` Mikael Magnusson 1 sibling, 1 reply; 36+ messages in thread From: Lawrence Velázquez @ 2015-01-09 18:33 UTC (permalink / raw) To: Ray Andrews; +Cc: zsh-workers On Jan 9, 2015, at 12:45 PM, Ray Andrews <rayandrews@eastlink.ca> wrote: > Holy Cow. That's just been sitting there for god knows how long? Since before zsh was under SourceForge version control, it looks like. % git blame -L 2179,+6 Src/hist.c(git:master) e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2179) *rptr++ = *ptr; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2180) *rptr++ = '\''; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2181) *rptr++ = 0; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2182) str[1] = NULL; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2183) return 0; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2184) } % git show --shortstat e74702b4(git:master) commit e74702b467171dbdafb56dfe354794a212e020d9 Author: Tanaka Akira <akr@users.sourceforge.net> Date: Thu Apr 15 18:05:38 1999 +0000 Initial revision 354 files changed, 66043 insertions(+) vq ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 18:33 ` Lawrence Velázquez @ 2015-01-09 18:36 ` Lawrence Velázquez 0 siblings, 0 replies; 36+ messages in thread From: Lawrence Velázquez @ 2015-01-09 18:36 UTC (permalink / raw) To: Ray Andrews; +Cc: zsh-workers On Jan 9, 2015, at 1:33 PM, Lawrence Velázquez <vq@larryv.me> wrote: > % git blame -L 2179,+6 Src/hist.c(git:master) > e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2179) *rptr++ = *ptr; > e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2180) *rptr++ = '\''; > e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2181) *rptr++ = 0; > e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2182) str[1] = NULL; > e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2183) return 0; > e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2184) } > % git show --shortstat e74702b4(git:master) > commit e74702b467171dbdafb56dfe354794a212e020d9 > Author: Tanaka Akira <akr@users.sourceforge.net> > Date: Thu Apr 15 18:05:38 1999 +0000 > > Initial revision > > 354 files changed, 66043 insertions(+) Badly-edited paste. In the interests of correctness: % git blame -L 2179,+6 Src/hist.c e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2179) *rptr++ = *ptr; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2180) *rptr++ = '\''; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2181) *rptr++ = 0; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2182) str[1] = NULL; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2183) return 0; e74702b4 (Tanaka Akira 1999-04-15 18:05:38 +0000 2184) } % git show --shortstat e74702b4 commit e74702b467171dbdafb56dfe354794a212e020d9 Author: Tanaka Akira <akr@users.sourceforge.net> Date: Thu Apr 15 18:05:38 1999 +0000 Initial revision 354 files changed, 66043 insertions(+) vq ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 17:45 ` Ray Andrews 2015-01-09 18:33 ` Lawrence Velázquez @ 2015-01-09 19:38 ` Mikael Magnusson 2015-01-09 21:39 ` Ray Andrews 1 sibling, 1 reply; 36+ messages in thread From: Mikael Magnusson @ 2015-01-09 19:38 UTC (permalink / raw) To: Ray Andrews; +Cc: zsh workers On Fri, Jan 9, 2015 at 6:45 PM, Ray Andrews <rayandrews@eastlink.ca> wrote: > On 01/09/2015 04:43 AM, Mikael Magnusson wrote: >> >> This actually writes a NULL to some arbitrary location in the caller >> function's stack. Found by Coverity (Issue 1255746). >> >> The start of the quote() function does char **str = tr; and is called like >> this, >> quote(&sline); >> sline in turn is just a char *sline; >> The result of str[1] = NULL; is then, as far as I can tell, not anything >> good. I also can't see any other thing that might have been intended to be >> NULL-terminated here, so just remove it. > > > Holy Cow. That's just been sitting there for god knows how long? Yes, but it's actually pretty harmless, most other variables on the stack in that function are never used if we enter this codepath. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 19:38 ` Mikael Magnusson @ 2015-01-09 21:39 ` Ray Andrews 2015-01-09 22:30 ` Peter Stephenson 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-09 21:39 UTC (permalink / raw) To: Mikael Magnusson; +Cc: zsh workers On 01/09/2015 11:38 AM, Mikael Magnusson wrote: > Yes, but it's actually pretty harmless, most other variables on the > stack in that function are never used if we enter this codepath. I guess so, since otherwise there'd be crashes all over the place. Still, it has a lethal look about it. Trying to understand the culture of the project I find myself wondering how things like that pass unnoticed, given the unquestioned skill level of the people involved. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 21:39 ` Ray Andrews @ 2015-01-09 22:30 ` Peter Stephenson 2015-01-09 23:39 ` Ray Andrews 0 siblings, 1 reply; 36+ messages in thread From: Peter Stephenson @ 2015-01-09 22:30 UTC (permalink / raw) To: zsh workers On Fri, 09 Jan 2015 13:39:06 -0800 Ray Andrews <rayandrews@eastlink.ca> wrote: > On 01/09/2015 11:38 AM, Mikael Magnusson wrote: > > > Yes, but it's actually pretty harmless, most other variables on the > > stack in that function are never used if we enter this codepath. > > I guess so, since otherwise there'd be crashes all over the place. Still, it > has a lethal look about it. Trying to understand the culture of the project > I find myself wondering how things like that pass unnoticed, given the > unquestioned skill level of the people involved. Bart already answered this once. There is no "magic away problems with the force of my intellect" in programming any more than anywhere else. There are things that happen to show up in real life; there's painstaking use of tools as Mikael is doing; and there are things you happen to notice because you were in the area. The same is true of pretty much all software apart from the very small subset that can be proved mathematically correct (and even that's only necessarily true of the representation of the software in the form you're looking at). The next best step is to use a better language than C. I wasn't entirely joking when I talked about D, but it's not an option for us. pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 22:30 ` Peter Stephenson @ 2015-01-09 23:39 ` Ray Andrews 2015-01-10 0:39 ` Bart Schaefer 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-09 23:39 UTC (permalink / raw) To: zsh-workers On 01/09/2015 02:30 PM, Peter Stephenson wrote: > Bart already answered this once. There is no "magic away problems with > the force of my intellect" in programming any more than anywhere else. Indeed not. > There are things that happen to show up in real life; there's > painstaking use of tools as Mikael is doing; Yes, it seems like fantastic work to me. > and there are things you happen to notice because you were in the > area. The same is true of pretty much all software apart from the very > small subset that can be proved mathematically correct (and even > that's only necessarily true of the representation of the software in > the form you're looking at). The next best step is to use a better > language than C. I wasn't entirely joking when I talked about D, but > it's not an option for us. pws Yeah, don't take it as me being bitchy Peter, it really is a question, not a veiled comment. As I've learned, zsh is not a huge corporation (like I thought it would be), and there's always been more coders than testers. And power seems to be valued above simplicity, and the code is 'layered' very heavily which makes things difficult to understand. Still, seeing an assignment to what turns out to be a dangling pointer (if I'm saying that correctly) raises my eyebrows. In the culture I come from, a good tester is valued above a good coder. It's me who has to adapt of course. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 23:39 ` Ray Andrews @ 2015-01-10 0:39 ` Bart Schaefer 2015-01-10 7:45 ` Ray Andrews 0 siblings, 1 reply; 36+ messages in thread From: Bart Schaefer @ 2015-01-10 0:39 UTC (permalink / raw) To: Zsh hackers list On Fri, Jan 9, 2015 at 3:39 PM, Ray Andrews <rayandrews@eastlink.ca> wrote: > > Yeah, don't take it as me being bitchy Peter, it really is a question, not a > veiled comment. Except it *is* a veiled comment, because you ask how "the culture" could allow this to "pass unnoticed" as if simply by having a different attitude we could have emulated thousands of lines of code in our heads to find all possible errors (even those that have no side effects, like this one) during the years that sophisticated automatic analysis tools were simply not available except possibly to well-funded corporate teams. Right. Just a question. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-10 0:39 ` Bart Schaefer @ 2015-01-10 7:45 ` Ray Andrews 2015-01-10 22:04 ` Lawrence Velázquez 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-10 7:45 UTC (permalink / raw) To: zsh-workers On 01/09/2015 04:39 PM, Bart Schaefer wrote: > Except it *is* a veiled comment, because you ask how "the culture" > could allow this to "pass unnoticed" No sir, it is a question. And I think the answer has been given, and it is that there has tended to be a shortage of testers. > as if simply by having a > different attitude we could have emulated thousands of lines of code > in our heads to find all possible errors ... and that's another part of it, that things are so complicated. > (even those that have no side ... and that it's not serious anyway. (In DOS, something like that would be a crasher.) > effects, like this one) during the years that sophisticated automatic > analysis tools were simply not available except possibly to > well-funded corporate teams. ... and that too: zsh is not a big project nor funded in any way. Quite astonishing what has been achieved, given all that, I'd say. And as to 'simply having a different attitude' ... I wish it was simple, look at the trouble I'm having ;-) I'm bugging people, better drop out of sight. > > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-10 7:45 ` Ray Andrews @ 2015-01-10 22:04 ` Lawrence Velázquez 2015-01-10 22:50 ` Ray Andrews 0 siblings, 1 reply; 36+ messages in thread From: Lawrence Velázquez @ 2015-01-10 22:04 UTC (permalink / raw) To: Ray Andrews; +Cc: zsh-workers On Jan 10, 2015, at 2:45 AM, Ray Andrews <rayandrews@eastlink.ca> wrote: > On 01/09/2015 04:39 PM, Bart Schaefer wrote: >> Except it *is* a veiled comment, because you ask how "the culture" >> could allow this to "pass unnoticed" > > No sir, it is a question. Surely you are familiar with the concept of connotation, whether intentional or not. Compare this… "This bug seems like it could have been caught sooner with better testing and code auditing. Is there a reason you don't do more of that? Here are some suggestions for making testing easier and more robust." …with… "WOW, this is a REALLY DUMB MISTAKE. How could a bunch of people who are *so smart* make such a _boneheaded_ blunder? It confuses me because I'm used to a development culture that puts software quality *first*. I'm not commenting on anything, just asking a question." If you think I'm exaggerating, go back and read what you wrote. It's not much of an exaggeration. vq ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-10 22:04 ` Lawrence Velázquez @ 2015-01-10 22:50 ` Ray Andrews 2015-01-11 1:58 ` Bart Schaefer 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-10 22:50 UTC (permalink / raw) To: Lawrence Velázquez; +Cc: zsh-workers On 01/10/2015 02:04 PM, Lawrence Velázquez wrote: > Surely you are familiar with the concept of connotation, whether > intentional or not. Compare this… > > "This bug seems like it could have been caught sooner with > better testing and code auditing. Is there a reason you don't do > more of that? Here are some suggestions for making testing > easier and more robust." > > …with… > > "WOW, this is a REALLY DUMB MISTAKE. How could a bunch of people > who are *so smart* make such a _boneheaded_ blunder? It confuses > me because I'm used to a development culture that puts software > quality *first*. I'm not commenting on anything, just asking > a question." > ...with... "In what environment does that sort of thing happen? Is it really that important anyway? How should I view this? We'd have considered that serious in DOS, but maybe it's a detail here. Looks like we need more testers? That's me! OTOH, it seems that zsh/linux code is just naturally more robust, maybe? Seems so. Is this 'bad' or just par for the course, I wonder. So many added layers! Must it be so? Perhaps I need a complete change in how I view a code defect." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-10 22:50 ` Ray Andrews @ 2015-01-11 1:58 ` Bart Schaefer 2015-01-11 5:46 ` Ray Andrews 0 siblings, 1 reply; 36+ messages in thread From: Bart Schaefer @ 2015-01-11 1:58 UTC (permalink / raw) To: zsh-workers On Jan 10, 2:50pm, Ray Andrews wrote: } } Looks like we need more testers? That's me! OK, I'll bite. Exactly what test would you have applied that would have found that stray assignment? Not that it wouldn't be good to have someone doing testing / writing new cases for Test/*.ztst. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-11 1:58 ` Bart Schaefer @ 2015-01-11 5:46 ` Ray Andrews 2015-01-11 7:10 ` Floating point modulus Bart Schaefer 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-11 5:46 UTC (permalink / raw) To: zsh-workers On 01/10/2015 05:58 PM, Bart Schaefer wrote: > On Jan 10, 2:50pm, Ray Andrews wrote: > } > } Looks like we need more testers? That's me! > > OK, I'll bite. Exactly what test would you have applied that would have > found that stray assignment? > > Not that it wouldn't be good to have someone doing testing / writing new > cases for Test/*.ztst. > I'm speaking in the most general terms, not about anything in particular. As I become more competent, I'll test the hell out of everything. I'd like to know how this Coverity thing works it seems fantastic. Speaking of testing, I can't get modulus to work nohow. $ echo $(( 2.5 )); 2.5 $ echo $(( 2.5 % 2 )); 0 ... If I'm doing something stupid there ... That's with force_float. Modulus with integers is fine. Other operators fine. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Floating point modulus 2015-01-11 5:46 ` Ray Andrews @ 2015-01-11 7:10 ` Bart Schaefer 2015-01-11 17:33 ` Peter Stephenson ` (2 more replies) 0 siblings, 3 replies; 36+ messages in thread From: Bart Schaefer @ 2015-01-11 7:10 UTC (permalink / raw) To: zsh-workers On Jan 10, 9:46pm, Ray Andrews wrote: } } Speaking of testing, I can't get modulus to work nohow. } } $ echo $(( 2.5 )); } 2.5 } } $ echo $(( 2.5 % 2 )); } 0 } } ... If I'm doing something stupid there ... Zsh implements the C modulus operator (%) directly, and it's defined only on integers. If you try that in C it won't even compile: error: invalid operands to binary % Consequently the operands are silently converted to integers, instead of the shell throwing an error. The expectation is that you will know that zsh math is C math (same as for why things are integer by default) and that you must do: $ zmodload zsh/mathfunc $ echo $(( fmod(2.5, 2) )) 0.5 The following patch would instead silently substitute the fmod() call when using the % operator on a float. I won't commit it without some further discussion from the group. With more effort it could be made to do this only when FORCE_FLOAT is in effect, though I'm not sure how many such special-cases we want to manage. diff --git a/Src/math.c b/Src/math.c index 438a170..6d096e0 100644 --- a/Src/math.c +++ b/Src/math.c @@ -288,11 +288,11 @@ static int type[TOKCOUNT] = { /* 0 */ LR, LR|OP_OP|OP_OPF, RL, RL, RL|OP_OP|OP_OPF, /* 5 */ RL|OP_OP|OP_OPF, RL, RL, LR|OP_A2IO, LR|OP_A2IO, -/* 10 */ LR|OP_A2IO, LR|OP_A2, LR|OP_A2, LR|OP_A2IO, LR|OP_A2, +/* 10 */ LR|OP_A2IO, LR|OP_A2, LR|OP_A2, LR|OP_A2, LR|OP_A2, /* 15 */ LR|OP_A2, LR|OP_A2IO, LR|OP_A2IO, LR|OP_A2IR, LR|OP_A2IR, /* 20 */ LR|OP_A2IR, LR|OP_A2IR, LR|OP_A2IR, LR|OP_A2IR, BOOL|OP_A2IO, /* 25 */ BOOL|OP_A2IO, LR|OP_A2IO, RL|OP_OP, RL|OP_OP, RL|OP_E2, -/* 30 */ RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2IO, +/* 30 */ RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, /* 35 */ RL|OP_E2IO, RL|OP_E2IO, RL|OP_E2IO, RL|OP_E2IO, RL|OP_E2IO, /* 40 */ BOOL|OP_E2IO, BOOL|OP_E2IO, RL|OP_A2IO, RL|OP_A2, RL|OP_OP, /* 45 */ RL, RL, LR|OP_OPF, LR|OP_OPF, RL|OP_A2, @@ -1133,7 +1133,9 @@ op(int what) * Any integer mod -1 is the same as any integer mod 1 * i.e. zero. */ - if (b.u.l == -1) + if (c.type == MN_FLOAT) + c.u.d = fmod(a.u.d, b.u.d); + else if (b.u.l == -1) c.u.l = 0; else c.u.l = a.u.l % b.u.l; ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 7:10 ` Floating point modulus Bart Schaefer @ 2015-01-11 17:33 ` Peter Stephenson 2015-01-11 19:25 ` Bart Schaefer 2015-01-11 19:25 ` Ray Andrews 2015-01-11 19:36 ` Bart Schaefer 2 siblings, 1 reply; 36+ messages in thread From: Peter Stephenson @ 2015-01-11 17:33 UTC (permalink / raw) To: zsh-workers On Sat, 10 Jan 2015 23:10:17 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > The following patch would instead silently substitute the fmod() call > when using the % operator on a float. Seems OK --- it avoids the very unusual implicit (at the shell rather than C level) cast from float back to integer. The only query is whether we need to test for HAVE_FMOD, though the manual page I can see suggests only the fmodf and fmodl variants we don't need are likely to be problematic. pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 17:33 ` Peter Stephenson @ 2015-01-11 19:25 ` Bart Schaefer 0 siblings, 0 replies; 36+ messages in thread From: Bart Schaefer @ 2015-01-11 19:25 UTC (permalink / raw) To: zsh-workers On Jan 11, 5:33pm, Peter Stephenson wrote: } } The only query is whether we need to test for HAVE_FMOD, though the } manual page I can see suggests only the fmodf and fmodl variants we } don't need are likely to be problematic. Wikipedia seems to agree with you. I'll commit as is and we can find out ... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 7:10 ` Floating point modulus Bart Schaefer 2015-01-11 17:33 ` Peter Stephenson @ 2015-01-11 19:25 ` Ray Andrews 2015-01-11 20:00 ` Bart Schaefer 2015-01-11 19:36 ` Bart Schaefer 2 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-11 19:25 UTC (permalink / raw) To: zsh-workers On 01/10/2015 11:10 PM, Bart Schaefer wrote: > Consequently the operands are silently converted to integers, instead > of the shell throwing an error. > > The expectation is that you will know that zsh math is C math (same > as for why things are integer by default) and that you must do: Expectation you say. Wouldn't it be polite for the docs to at least mention that? Modulo is in a list with other standard operators, no indication whatsoever that it has different limitations. And, given that modulo, even with floating point, is (dare I say) near the simplest of arithmetic operations, I'd have thought that floating point modulo would have been near the first thing implemented. mod-you-loh () { setopt force_float echo "Your number is: $1" echo "Your modulator (what is the proper word for that?) is: $2" remainder=$1 while [[ $remainder > $2 ]]; do let remainder=$remainder-$2 echo "Chewing away ... $remainder" done echo "All done, the remainder is: $remainder" } $ mod-you-loh 7.5 2 Your number is: 7.5 Your modulator (what is the proper word for that?) is: 2 Chewing away ... 5.5 Chewing away ... 3.5 Chewing away ... 1.5 All done, the remainder is: 1.5 ... even I can do it in three minutes. But it's not built in? <anecdote> Having zsh math brought to mind, I was thinking where I could use it in my own functions, and the first thing that came to mind is a place where modulo would be just the thing. (I am currently using something like my pretend function above). How nice to be able to forget the function and have all that power in one symbol: " % ". Nope. Integers only. :( </> > $ zmodload zsh/mathfunc > $ echo $(( fmod(2.5, 2) )) > 0.5 > > The following patch would instead silently substitute the fmod() call > when using the % operator on a float. I won't commit it without some > further discussion from the group. With more effort it could be made > to do this only when FORCE_FLOAT is in effect, though I'm not sure > how many such special-cases we want to manage. > > > diff --git a/Src/math.c b/Src/math.c > index 438a170..6d096e0 100644 > --- a/Src/math.c > +++ b/Src/math.c > @@ -288,11 +288,11 @@ static int type[TOKCOUNT] = > { > /* 0 */ LR, LR|OP_OP|OP_OPF, RL, RL, RL|OP_OP|OP_OPF, > /* 5 */ RL|OP_OP|OP_OPF, RL, RL, LR|OP_A2IO, LR|OP_A2IO, > -/* 10 */ LR|OP_A2IO, LR|OP_A2, LR|OP_A2, LR|OP_A2IO, LR|OP_A2, > +/* 10 */ LR|OP_A2IO, LR|OP_A2, LR|OP_A2, LR|OP_A2, LR|OP_A2, > /* 15 */ LR|OP_A2, LR|OP_A2IO, LR|OP_A2IO, LR|OP_A2IR, LR|OP_A2IR, > /* 20 */ LR|OP_A2IR, LR|OP_A2IR, LR|OP_A2IR, LR|OP_A2IR, BOOL|OP_A2IO, > /* 25 */ BOOL|OP_A2IO, LR|OP_A2IO, RL|OP_OP, RL|OP_OP, RL|OP_E2, > -/* 30 */ RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2IO, > +/* 30 */ RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, > /* 35 */ RL|OP_E2IO, RL|OP_E2IO, RL|OP_E2IO, RL|OP_E2IO, RL|OP_E2IO, > /* 40 */ BOOL|OP_E2IO, BOOL|OP_E2IO, RL|OP_A2IO, RL|OP_A2, RL|OP_OP, > /* 45 */ RL, RL, LR|OP_OPF, LR|OP_OPF, RL|OP_A2, I need to go and have a little cry. > @@ -1133,7 +1133,9 @@ op(int what) > * Any integer mod -1 is the same as any integer mod 1 > * i.e. zero. > */ > - if (b.u.l == -1) > + if (c.type == MN_FLOAT) > + c.u.d = fmod(a.u.d, b.u.d); > + else if (b.u.l == -1) > c.u.l = 0; > else > c.u.l = a.u.l % b.u.l; Gentlemen. Perhaps it's presumptuous of me to discuss this, but could a time not come when zsh is no longer chained to past practice? When decisions are based on merit, not on what ksh '79 did? Just for argument's sake, supposing zsh 6.0 was the breakout version. Everything in it is there on merit. Floating point math there by default! 1/2 + 1/2 = 1! Shocking, unprecedented, revolutionary! No rc compatibility. Mind, it's not that good things learned from rc would go away, it's just that they'd be there simply because they are good things and they might be modified to be even better, *even* if that was no longer rc compatible. Instead of having to drive forward while looking backwards out the rear window, imagine driving forward looking forward. Never forgetting the past, but not being limited by it either. Honestly now: how many converts do we get from csh anymore? Haven't most of them already come in from the cold? I suggest that the future of zsh is not with welcoming in the few remaining csh long beards, but by making the shell more usable for brand new people. People who expect 1/2 + 1/2 = 1. People familiar with electronic calculators. "Zsh 6.0 ... the shell of the future!" No? Even bash converts--we might think they come over because zsh is compatible, but perhaps even more would come over because it *isn't* compatible, it's better. Which is to say that if bash has some silliness, zsh would pointedly *not* have it no matter how venerable the silliness may be. Zsh already does that on several points. Ah have a dream today. People say the future is with Python and it's 'modern' cousins. Maybe, but shells will always be there, so why don't we make a forward looking one? I'm not saying that the ancient scrolls should be burned just that they no longer 'control' or limit or enforce counterproductive traditions on us. Battle cry: " 1/2 + 1/2 = 1 ". ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 19:25 ` Ray Andrews @ 2015-01-11 20:00 ` Bart Schaefer 2015-01-11 20:58 ` Ray Andrews 0 siblings, 1 reply; 36+ messages in thread From: Bart Schaefer @ 2015-01-11 20:00 UTC (permalink / raw) To: zsh-workers On Jan 11, 11:25am, Ray Andrews wrote: } } Expectation you say. Wouldn't it be polite for the docs to at least } mention that? The documentation has long been a sore spot, because it was originally written to only cover things that were different about zsh (from other shells) because it was expected to be the second shell adopted by an experienced user. There are a huge number of nooks and crannies to fill to make it comprehensively about everything zsh in a vacuum, and no dedicated documentation project/author. } Perhaps it's presumptuous of me to discuss this, but could a } time not come when zsh is no longer chained to past practice? When } decisions are based on merit, not on what ksh '79 did? Assuming I agree (which I don't) with your implication that decisions are not already based on merit: so far you haven't convinced me that this would result in anything other than a lot of arguments about "merit." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 20:00 ` Bart Schaefer @ 2015-01-11 20:58 ` Ray Andrews 2015-01-11 21:34 ` Peter Stephenson 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-11 20:58 UTC (permalink / raw) To: zsh-workers On 01/11/2015 12:00 PM, Bart Schaefer wrote: > On Jan 11, 11:25am, Ray Andrews wrote: > } > } Expectation you say. Wouldn't it be polite for the docs to at least > } mention that? > > The documentation has long been a sore spot, because it was originally > written to only cover things that were different about zsh (from other > shells) You know, that is one of those sentences you make every now and then that turns an entire fog bank into clear air. *now* I understand how to understand why the doc seems so faulty ... it was/is an addendum! And it's been patched up ever since, but never was a 'full document' from it's beginning, so of course it's full of gaps. > } Perhaps it's presumptuous of me to discuss this, but could a > } time not come when zsh is no longer chained to past practice? When > } decisions are based on merit, not on what ksh '79 did? > > Assuming I agree (which I don't) with your implication that decisions are > not already based on merit: so far you haven't convinced me that this > would result in anything other than a lot of arguments about "merit." Well, no doubt there would be some judgment calls on merit, no avoiding that. But since I've been involved ... actually the current issue is a small but probably perfect test case. If one ignored past practice and was concerned with maximum utility and most intuitive implementation, and one was designing a shell that had a built in calculator, and one was to decide what the answer should be to this expression: 1/2 + 1/2 = ... I think we'd all agree the answer is '1'. Why isn't it? Because of past practice. Thus Google's Library of Babel is plugged up with people asking over and over again why shell arithmetic seems to be busted. It's busted because of past practice. The justification for leaving it busted is just that it has always been busted, so should always remain busted. Is that merit? I'll bet anyone 50 cents that for every person who wants 1/2 + 1/2 = 0, there are a thousand people who want it to equal one. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 20:58 ` Ray Andrews @ 2015-01-11 21:34 ` Peter Stephenson 2015-01-12 0:18 ` Ray Andrews 0 siblings, 1 reply; 36+ messages in thread From: Peter Stephenson @ 2015-01-11 21:34 UTC (permalink / raw) To: zsh-workers On Sun, 11 Jan 2015 12:58:23 -0800 Ray Andrews <rayandrews@eastlink.ca> wrote: > 1/2 + 1/2 = > > ... I think we'd all agree the answer is '1'. Why isn't it? Because > of past practice. This is complete nonsense and Bart already took the time to explain at length what is going on here and why. I'm not going to repeat it. The idea that "we'd all agree that..." implies (i) you don't realise that many of the people round here have been doing some of programming, shell or whatever, for a long time, and don't agree anything of the sort (ii) you simply can't even be bothered to read the explanations repeatedly given of why the world isn't the way you want it to be and why it can't be coerced by shouting at it. I'd strongly suggest at this point you take a couple of months out to learn a bit about programming languages, of which zsh is an example (and not actually a very good introduction in many ways as a bit of a hybrid) by actually doing something with them and a good book of exmples. Otherwise the sheer wasted time is getting to be quite ridiculous. pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 21:34 ` Peter Stephenson @ 2015-01-12 0:18 ` Ray Andrews 2015-01-12 10:03 ` Peter Stephenson 0 siblings, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-12 0:18 UTC (permalink / raw) To: zsh-workers On 01/11/2015 01:34 PM, Peter Stephenson wrote: > Otherwise the sheer wasted time is getting to be quite ridiculous. pws As you wish sir. I'm gone. I'll continue to do a daily build and report anything that seems to have broken, but I will neither ask questions nor make comments. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-12 0:18 ` Ray Andrews @ 2015-01-12 10:03 ` Peter Stephenson 0 siblings, 0 replies; 36+ messages in thread From: Peter Stephenson @ 2015-01-12 10:03 UTC (permalink / raw) To: zsh-workers On Sun, 11 Jan 2015 16:18:22 -0800 Ray Andrews <rayandrews@eastlink.ca> wrote: > On 01/11/2015 01:34 PM, Peter Stephenson wrote: > > Otherwise the sheer wasted time is getting to be quite ridiculous. pws > > As you wish sir. I'm gone. I'll continue to do a daily build and report > anything that seems to have broken, but I will neither ask questions > nor make comments. Questions are fine, and might well point to real issues. The problem is what's been ensuing when the answer comes back. I won't labour the point, though. pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 7:10 ` Floating point modulus Bart Schaefer 2015-01-11 17:33 ` Peter Stephenson 2015-01-11 19:25 ` Ray Andrews @ 2015-01-11 19:36 ` Bart Schaefer 2015-01-11 20:01 ` Peter Stephenson 2015-01-12 11:36 ` Vincent Lefevre 2 siblings, 2 replies; 36+ messages in thread From: Bart Schaefer @ 2015-01-11 19:36 UTC (permalink / raw) To: zsh-workers On Jan 10, 11:10pm, Bart Schaefer wrote: } } The following patch would instead silently substitute the fmod() call } when using the % operator on a float. Incidentally, what's a reliable test for this that could be added to C01arith.ztst? I worry about rounding error differing by platform: torch% print $(( 29.1 % 13.0 )) 3.1000000000000014 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 19:36 ` Bart Schaefer @ 2015-01-11 20:01 ` Peter Stephenson 2015-01-11 20:04 ` Bart Schaefer 2015-01-12 11:36 ` Vincent Lefevre 1 sibling, 1 reply; 36+ messages in thread From: Peter Stephenson @ 2015-01-11 20:01 UTC (permalink / raw) To: zsh-workers On Sun, 11 Jan 2015 11:36:01 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 10, 11:10pm, Bart Schaefer wrote: > } The following patch would instead silently substitute the fmod() call > } when using the % operator on a float. > > Incidentally, what's a reliable test for this that could be added to > C01arith.ztst? I worry about rounding error differing by platform: > > torch% print $(( 29.1 % 13.0 )) > 3.1000000000000014 diff --git a/Test/V03mathfunc.ztst b/Test/V03mathfunc.ztst index ab383db..8761252 100644 --- a/Test/V03mathfunc.ztst +++ b/Test/V03mathfunc.ztst @@ -142,3 +142,7 @@ F:This test fails if your math library doesn't have erand48(). print $g, $l 0:Test Gamma function gamma and lgamma >1.00000, 0.00000 + + print $(( int((29.1 % 13.0 * 10) + 0.5) )) +0:Test floating point modulo function +>31 pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 20:01 ` Peter Stephenson @ 2015-01-11 20:04 ` Bart Schaefer 2015-01-11 20:25 ` Peter Stephenson 0 siblings, 1 reply; 36+ messages in thread From: Bart Schaefer @ 2015-01-11 20:04 UTC (permalink / raw) To: zsh-workers On Jan 11, 8:01pm, Peter Stephenson wrote: } } diff --git a/Test/V03mathfunc.ztst b/Test/V03mathfunc.ztst } index ab383db..8761252 100644 } --- a/Test/V03mathfunc.ztst } +++ b/Test/V03mathfunc.ztst Thanks, but V03mathfunc is skipped if the zsh/mathfunc module is not loadable -- so this test should be in C01arith.ztst. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 20:04 ` Bart Schaefer @ 2015-01-11 20:25 ` Peter Stephenson 2015-01-12 0:02 ` Ray Andrews 2015-01-12 2:46 ` Bart Schaefer 0 siblings, 2 replies; 36+ messages in thread From: Peter Stephenson @ 2015-01-11 20:25 UTC (permalink / raw) To: zsh-workers On Sun, 11 Jan 2015 12:04:23 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 11, 8:01pm, Peter Stephenson wrote: > } > } diff --git a/Test/V03mathfunc.ztst b/Test/V03mathfunc.ztst > } index ab383db..8761252 100644 > } --- a/Test/V03mathfunc.ztst > } +++ b/Test/V03mathfunc.ztst > > Thanks, but V03mathfunc is skipped if the zsh/mathfunc module is not > loadable -- so this test should be in C01arith.ztst. I'll leave that to you, but instead of an explicit rounding you could do basically the same calculation but assigned to a variable declared as an integer and output that.xs pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 20:25 ` Peter Stephenson @ 2015-01-12 0:02 ` Ray Andrews 2015-01-12 2:23 ` Bart Schaefer 2015-01-12 2:46 ` Bart Schaefer 1 sibling, 1 reply; 36+ messages in thread From: Ray Andrews @ 2015-01-12 0:02 UTC (permalink / raw) To: zsh-workers On 01/11/2015 12:25 PM, Peter Stephenson wrote: > I'll leave that to you, but instead of an explicit rounding you could > do basically the same calculation but assigned to a variable declared > as an integer and output that.xs pws remainder=$1 Ignore this question if it can't be explained simply, since I don't have the knowledge probably to even understand the answer, but why is it more complicated than (for example) this: while [[ $remainder > $2 ]]; do let remainder=$remainder-$2 done ... can't that sorta be attached to " % " without dozens of lines of changed code? Seems not :( And: /* 25 */ BOOL|OP_A2IO, LR|OP_A2IO, RL|OP_OP, RL|OP_OP, RL|OP_E2, -/* 30 */ RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2IO, ... are those 'metas'? IOW, is that the internal representation of some command syntax? Metafied? Tokenized? ... > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-12 0:02 ` Ray Andrews @ 2015-01-12 2:23 ` Bart Schaefer 0 siblings, 0 replies; 36+ messages in thread From: Bart Schaefer @ 2015-01-12 2:23 UTC (permalink / raw) To: zsh-workers On Jan 11, 4:02pm, Ray Andrews wrote: } Subject: Re: Floating point modulus } } On 01/11/2015 12:25 PM, Peter Stephenson wrote: } } > I'll leave that to you, but instead of an explicit rounding you could } > do basically the same calculation but assigned to a variable declared } > as an integer and output that.xs pws remainder=$1 } } Ignore this question if it can't be explained simply, since } I don't have the knowledge probably to even understand the } answer, but why is it more complicated You're conflating two only loosely related things. Above, Peter is talking about the regression test shell code; you seem to be asking about the C code change, which already works fine -- except that on different hardware/OS platforms, different implementations of double- precision arithmetic may cause the same floating-point operation to return results that differ in the less-significant decimal places. Since the tests rely on being able to (textually) compare an actual result to an expected result, you can't test just any floating point expression, you have to choose a floating point expression that will be the same everywhere to however many decimal places. } And: } } /* 25 */ BOOL|OP_A2IO, LR|OP_A2IO, RL|OP_OP, RL|OP_OP, RL|OP_E2, } -/* 30 */ RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2, RL|OP_E2IO, } } ... are those 'metas'? IOW, is that the internal representation of } some command syntax? Metafied? Tokenized? ... There are two tables in math.c, one with symbolic names of all the operators, and one with symbolic (represented as bitfield) binding and precedence of those same operators. The tables line up positionally. Yes, this should probably have been done with structs instead of with two parallel arrays, but this code was written by a college sophomore in 1993 or so and barely changed since. LR and RL are binding, OP_A2 and OP_E2 are assignment or expression and the number of operands, and a suffix of IO means the operator is "integer only". This information is used by the math parser. So my patch removed the "integer only" bit from the map for % and %= and changed the actual implemention to call fmod() for non-integers. The change is very small compared to the diff context produced. -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 20:25 ` Peter Stephenson 2015-01-12 0:02 ` Ray Andrews @ 2015-01-12 2:46 ` Bart Schaefer 2015-01-12 9:56 ` Peter Stephenson 1 sibling, 1 reply; 36+ messages in thread From: Bart Schaefer @ 2015-01-12 2:46 UTC (permalink / raw) To: zsh-workers On Jan 11, 8:25pm, Peter Stephenson wrote: } } I'll leave that to you, but instead of an explicit rounding you could do } basically the same calculation but assigned to a variable declared as an } integer and output that. Hm. This seems like a bug: % integer rnd % print -- $(( rnd = ((29.1 % 13.0 * 10) + 0.5) )) 31.500000000000014 Shouldn't the result of the $(( ... )) be an integer because rnd has been declared as an integer? I had to do this instead: diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst index 59d182a..8da17f7 100644 --- a/Test/C01arith.ztst +++ b/Test/C01arith.ztst @@ -18,6 +18,12 @@ 0:basic floating point arithmetic >31415. + integer rnd + (( rnd = ((29.1 % 13.0 * 10) + 0.5) )) + print $rnd +0:Test floating point modulo function +>31 + print $(( 0x10 + 0X01 + 2#1010 )) 0:base input >27 -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-12 2:46 ` Bart Schaefer @ 2015-01-12 9:56 ` Peter Stephenson 2015-01-12 13:49 ` Peter Stephenson 2015-01-12 16:35 ` Bart Schaefer 0 siblings, 2 replies; 36+ messages in thread From: Peter Stephenson @ 2015-01-12 9:56 UTC (permalink / raw) To: zsh-workers On Sun, 11 Jan 2015 18:46:32 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 11, 8:25pm, Peter Stephenson wrote: > } > } I'll leave that to you, but instead of an explicit rounding you could do > } basically the same calculation but assigned to a variable declared as an > } integer and output that. > > Hm. This seems like a bug: > > % integer rnd > % print -- $(( rnd = ((29.1 % 13.0 * 10) + 0.5) )) > 31.500000000000014 > > Shouldn't the result of the $(( ... )) be an integer because rnd has been > declared as an integer? The result above is how I always thought it worked, actually. The type of the result isn't necessarily the type of the variable assigned to; there's never been any code to propagate types back from assignments, unlike in C. Whether or not it should be is another question. Looking at the doc, with its references to C, you'd probably be entitled to expect assignment types propagated the way they do in C. In particular: % integer foo % (( foo )); print $? 1 # foo is zero, status is non-zero % (( foo = 3.5/5.5 )); print $? 0 # foo is zero but # status is also zero as # result is non zero % (( foo )); print $? 1 # different result You might think the status at least went with the assignment. > I had to do this instead: That's what I was expecting you to do. pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-12 9:56 ` Peter Stephenson @ 2015-01-12 13:49 ` Peter Stephenson 2015-01-12 16:35 ` Bart Schaefer 1 sibling, 0 replies; 36+ messages in thread From: Peter Stephenson @ 2015-01-12 13:49 UTC (permalink / raw) To: zsh-workers On Mon, 12 Jan 2015 09:56:28 +0000 Peter Stephenson <p.stephenson@samsung.com> wrote: > On Sun, 11 Jan 2015 18:46:32 -0800 > Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Jan 11, 8:25pm, Peter Stephenson wrote: > > } > > } I'll leave that to you, but instead of an explicit rounding you could do > > } basically the same calculation but assigned to a variable declared as an > > } integer and output that. > > > > Hm. This seems like a bug: > > > > % integer rnd > > % print -- $(( rnd = ((29.1 % 13.0 * 10) + 0.5) )) > > 31.500000000000014 > > > > Shouldn't the result of the $(( ... )) be an integer because rnd has been > > declared as an integer? > > Whether or not it should be is another question. Looking at the doc, > with its references to C, you'd probably be entitled to expect assignment > types propagated the way they do in C. This appears to be straightforward. It looks like we were actually assuming this behaviour in one existing test, at least in the code, but then treating the result as floating point anway. Duh. I've added some notes to the README about the change. pws diff --git a/README b/README index e3ccc70..977f3fa 100644 --- a/README +++ b/README @@ -35,6 +35,54 @@ Zsh is a shell with lots of features. For a list of some of these, see the file FEATURES, and for the latest changes see NEWS. For more details, see the documentation. +Incompatibilites between 5.0.7 and 5.0.8 +---------------------------------------- + +A couple of arithmetic operations have changed: the new behaviour +is intended to be more consistent, but is not compatible with the old. + +Previously, the modulus operation, `%', implicitly converted the +operation to integer and output an integer result, even if one +or both of the arguments were floating point. Now, the C math +library fmod() operator is used to implement the operation where +one of the arguments is floating point. For example: + +Old behavour: + +% print $(( 5.5 % 2 )) +1 + +New behaviour: + +% print $(( 5.5 % 2 )) +1.5 + +Previously, assignments to variables assigned the correct type to +variables declared as floating point or integer, but this type was +not propagated to the value of the expression, as a C programmer +would naturally expect. Now, the type of the variable is propagated +so long as the variable is declared as a numeric type (however this +happened, e.g. the variable may have been implicitly typed by a +previous assignment). For example: + +Old behaviour: + +% integer var +% print $(( var = 5.5 / 2.0 )) +2.2000000000000002 +% print $var +2 + +(the actual rounding error may vary). + +New behaviour: + +% integer var +% print $(( var = 5.5 / 2.0 )) +2 +% print $var +2 + Incompatibilities between 5.0.2 and 5.0.5 ----------------------------------------- diff --git a/Src/math.c b/Src/math.c index 6d096e0..db42d0f 100644 --- a/Src/math.c +++ b/Src/math.c @@ -880,6 +880,8 @@ getcvar(char *s) static mnumber setmathvar(struct mathvalue *mvp, mnumber v) { + Param pm; + if (mvp->pval) { /* * This value may have been hanging around for a while. @@ -909,7 +911,32 @@ setmathvar(struct mathvalue *mvp, mnumber v) if (noeval) return v; untokenize(mvp->lval); - setnparam(mvp->lval, v); + pm = setnparam(mvp->lval, v); + if (pm) { + /* + * If we are performing an assignment, we return the + * number with the same type as the parameter we are + * assigning to, in the spirit of the way assignments + * in C work. Note this was a change to long-standing + * zsh behaviour. + */ + switch (PM_TYPE(pm->node.flags)) { + case PM_INTEGER: + if (v.type != MN_INTEGER) { + v.u.l = (zlong)v.u.d; + v.type = MN_INTEGER; + } + break; + + case PM_EFLOAT: + case PM_FFLOAT: + if (v.type != MN_FLOAT) { + v.u.d = (double)v.u.l; + v.type = MN_FLOAT; + } + break; + } + } return v; } diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst index 8da17f7..8e0730d 100644 --- a/Test/C01arith.ztst +++ b/Test/C01arith.ztst @@ -16,7 +16,7 @@ print -- $(( rnd = there * 10000 )) # save rounding problems by converting to integer 0:basic floating point arithmetic ->31415. +>31415 integer rnd (( rnd = ((29.1 % 13.0 * 10) + 0.5) )) @@ -300,3 +300,11 @@ print $(( 0b2 )) 1:Binary numbers don't tend to have 2's in ?(eval):1: bad math expression: operator expected at `2 ' + + integer varassi + print $(( varassi = 5.5 / 2.0 )) + print $varassi +0:Integer variable assignment converts result to integer +>2 +>2 +# It's hard to test for integer to float. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-12 9:56 ` Peter Stephenson 2015-01-12 13:49 ` Peter Stephenson @ 2015-01-12 16:35 ` Bart Schaefer 2015-01-12 16:45 ` Peter Stephenson 1 sibling, 1 reply; 36+ messages in thread From: Bart Schaefer @ 2015-01-12 16:35 UTC (permalink / raw) To: zsh-workers On Jan 12, 9:56am, Peter Stephenson wrote: } } > I had to do this instead: } } That's what I was expecting you to do. I see you've already done something with this, but just to close the thread: I thought you were expecting me to do what was done in the immediately preceding test [ print -- $(( rnd = ... )) ] and was surprised when it didn't work. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-12 16:35 ` Bart Schaefer @ 2015-01-12 16:45 ` Peter Stephenson 0 siblings, 0 replies; 36+ messages in thread From: Peter Stephenson @ 2015-01-12 16:45 UTC (permalink / raw) To: zsh-workers On Mon, 12 Jan 2015 08:35:57 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 12, 9:56am, Peter Stephenson wrote: > } > } > I had to do this instead: > } > } That's what I was expecting you to do. > > I see you've already done something with this, but just to close the > thread: I thought you were expecting me to do what was done in the > immediately preceding test [ print -- $(( rnd = ... )) ] and was > surprised when it didn't work. Yes, that's the test I noted we (probably I) had screwed up over when I wrote the last patch, so the previous situation was indeed a bit confused. Should all be consistent now. pws ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Floating point modulus 2015-01-11 19:36 ` Bart Schaefer 2015-01-11 20:01 ` Peter Stephenson @ 2015-01-12 11:36 ` Vincent Lefevre 1 sibling, 0 replies; 36+ messages in thread From: Vincent Lefevre @ 2015-01-12 11:36 UTC (permalink / raw) To: zsh-workers On 2015-01-11 11:36:01 -0800, Bart Schaefer wrote: > On Jan 10, 11:10pm, Bart Schaefer wrote: > } > } The following patch would instead silently substitute the fmod() call > } when using the % operator on a float. > > Incidentally, what's a reliable test for this that could be added to > C01arith.ztst? I worry about rounding error differing by platform: > > torch% print $(( 29.1 % 13.0 )) > 3.1000000000000014 The rounding error is due to the decimal-to-binary conversions, not to the % operation, which, if I'm not mistaken, is necessarily exact in a floating-point system: considering x % y, if |x| < |y|, then the result is x, which is exact; otherwise the result is a multiple of ulp(y) and less than |y| in absolute value, thus exactly representable. -- Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: PATCH: hist: remove wrong NULL terminator 2015-01-09 12:43 PATCH: hist: remove wrong NULL terminator Mikael Magnusson 2015-01-09 17:45 ` Ray Andrews @ 2015-01-10 7:08 ` Bart Schaefer 1 sibling, 0 replies; 36+ messages in thread From: Bart Schaefer @ 2015-01-10 7:08 UTC (permalink / raw) To: zsh-workers On Jan 9, 1:43pm, Mikael Magnusson wrote: } Subject: PATCH: hist: remove wrong NULL terminator } } The result of str[1] = NULL; is then, as far as I can tell, not } anything good. I also can't see any other thing that might have been } intended to be NULL-terminated here, so just remove it. That whole function is a bit odd. It's exported, but nothing calls it except in this file. My compiler doesn't complain if changed to static. And both it and quotebreak() start like quote(char **tr) { char *ptr, *rptr, **str = (char **)tr; what is the cast for if tr is already char** ? Anyway I don't see any reason not to commit the patch from 34178. ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2015-01-12 16:55 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-09 12:43 PATCH: hist: remove wrong NULL terminator Mikael Magnusson 2015-01-09 17:45 ` Ray Andrews 2015-01-09 18:33 ` Lawrence Velázquez 2015-01-09 18:36 ` Lawrence Velázquez 2015-01-09 19:38 ` Mikael Magnusson 2015-01-09 21:39 ` Ray Andrews 2015-01-09 22:30 ` Peter Stephenson 2015-01-09 23:39 ` Ray Andrews 2015-01-10 0:39 ` Bart Schaefer 2015-01-10 7:45 ` Ray Andrews 2015-01-10 22:04 ` Lawrence Velázquez 2015-01-10 22:50 ` Ray Andrews 2015-01-11 1:58 ` Bart Schaefer 2015-01-11 5:46 ` Ray Andrews 2015-01-11 7:10 ` Floating point modulus Bart Schaefer 2015-01-11 17:33 ` Peter Stephenson 2015-01-11 19:25 ` Bart Schaefer 2015-01-11 19:25 ` Ray Andrews 2015-01-11 20:00 ` Bart Schaefer 2015-01-11 20:58 ` Ray Andrews 2015-01-11 21:34 ` Peter Stephenson 2015-01-12 0:18 ` Ray Andrews 2015-01-12 10:03 ` Peter Stephenson 2015-01-11 19:36 ` Bart Schaefer 2015-01-11 20:01 ` Peter Stephenson 2015-01-11 20:04 ` Bart Schaefer 2015-01-11 20:25 ` Peter Stephenson 2015-01-12 0:02 ` Ray Andrews 2015-01-12 2:23 ` Bart Schaefer 2015-01-12 2:46 ` Bart Schaefer 2015-01-12 9:56 ` Peter Stephenson 2015-01-12 13:49 ` Peter Stephenson 2015-01-12 16:35 ` Bart Schaefer 2015-01-12 16:45 ` Peter Stephenson 2015-01-12 11:36 ` Vincent Lefevre 2015-01-10 7:08 ` PATCH: hist: remove wrong NULL terminator Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).