2009-12-27 13:19:00

by Roel Kluin

Subject: [PATCH] parisc: test off by one in sgl_frem() and dbl_frem()

With `while (stepcount-- > 0)' stepcount reaches -1 after the loop.

Signed-off-by: Roel Kluin <[email protected]>
---
arch/parisc/math-emu/dfrem.c | 2 +-
arch/parisc/math-emu/sfrem.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

Unless I am missing something?

diff --git a/arch/parisc/math-emu/dfrem.c b/arch/parisc/math-emu/dfrem.c
index b983785..3283445 100644
--- a/arch/parisc/math-emu/dfrem.c
+++ b/arch/parisc/math-emu/dfrem.c
@@ -234,7 +234,7 @@ dbl_frem (dbl_floating_point * srcptr1, dbl_floating_point * srcptr2,
Dbl_subtract(opnd1p1,opnd1p2,opnd2p1,opnd2p2,opnd1p1,opnd1p2);
roundup = TRUE;
}
- if (stepcount > 0 || Dbl_iszero(opnd1p1,opnd1p2)) {
+ if (stepcount >= 0 || Dbl_iszero(opnd1p1,opnd1p2)) {
/* division is exact, remainder is zero */
Dbl_setzero_exponentmantissa(resultp1,resultp2);
Dbl_copytoptr(resultp1,resultp2,dstptr);
diff --git a/arch/parisc/math-emu/sfrem.c b/arch/parisc/math-emu/sfrem.c
index 3a1b7a3..ad87832 100644
--- a/arch/parisc/math-emu/sfrem.c
+++ b/arch/parisc/math-emu/sfrem.c
@@ -229,7 +229,7 @@ sgl_frem (sgl_floating_point * srcptr1, sgl_floating_point * srcptr2,
Sgl_subtract(opnd1,opnd2,opnd1);
roundup = TRUE;
}
- if (stepcount > 0 || Sgl_iszero(opnd1)) {
+ if (stepcount >= 0 || Sgl_iszero(opnd1)) {
/* division is exact, remainder is zero */
Sgl_setzero_exponentmantissa(result);
*dstptr = result;

2009-12-27 15:17:54

by John David Anglin

Subject: Re: [PATCH] parisc: test off by one in sgl_frem() and dbl_frem()

> With `while (stepcount-- > 0)' stepcount reaches -1 after the loop.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> arch/parisc/math-emu/dfrem.c | 2 +-
> arch/parisc/math-emu/sfrem.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> Unless I am missing something?

Are you sure the correct fix isn't to move the decrement of stepcount
into the loop?

Dave
--
J. David Anglin [email protected]
National Research Council of Canada (613) 990-0752 (FAX: 952-6602)

2009-12-27 15:30:13

by James Bottomley

Subject: Re: [PATCH] parisc: test off by one in sgl_frem() and dbl_frem()

On Sun, 2009-12-27 at 14:21 +0100, Roel Kluin wrote:
> With `while (stepcount-- > 0)' stepcount reaches -1 after the loop.

This is true, but seems to be by design

> Signed-off-by: Roel Kluin <[email protected]>
> ---
> arch/parisc/math-emu/dfrem.c | 2 +-
> arch/parisc/math-emu/sfrem.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> Unless I am missing something?
>
> diff --git a/arch/parisc/math-emu/dfrem.c b/arch/parisc/math-emu/dfrem.c
> index b983785..3283445 100644
> --- a/arch/parisc/math-emu/dfrem.c
> +++ b/arch/parisc/math-emu/dfrem.c
> @@ -234,7 +234,7 @@ dbl_frem (dbl_floating_point * srcptr1, dbl_floating_point * srcptr2,
> Dbl_subtract(opnd1p1,opnd1p2,opnd2p1,opnd2p2,opnd1p1,opnd1p2);
> roundup = TRUE;
> }
> - if (stepcount > 0 || Dbl_iszero(opnd1p1,opnd1p2)) {
> + if (stepcount >= 0 || Dbl_iszero(opnd1p1,opnd1p2)) {
> /* division is exact, remainder is zero */
> Dbl_setzero_exponentmantissa(resultp1,resultp2);
> Dbl_copytoptr(resultp1,resultp2,dstptr);
> diff --git a/arch/parisc/math-emu/sfrem.c b/arch/parisc/math-emu/sfrem.c
> index 3a1b7a3..ad87832 100644
> --- a/arch/parisc/math-emu/sfrem.c
> +++ b/arch/parisc/math-emu/sfrem.c
> @@ -229,7 +229,7 @@ sgl_frem (sgl_floating_point * srcptr1, sgl_floating_point * srcptr2,
> Sgl_subtract(opnd1,opnd2,opnd1);
> roundup = TRUE;
> }
> - if (stepcount > 0 || Sgl_iszero(opnd1)) {
> + if (stepcount >= 0 || Sgl_iszero(opnd1)) {
> /* division is exact, remainder is zero */
> Sgl_setzero_exponentmantissa(result);
> *dstptr = result;

Your patch does nothing to the actual execution flow (Sgl_iszero is true
if stepcount == 0) ... what's the point of applying it?

James

2009-12-27 21:08:17

by Roel Kluin

Subject: Re: [PATCH] parisc: test off by one in sgl_frem() and dbl_frem()

Op 27-12-09 16:30, James Bottomley schreef:
> On Sun, 2009-12-27 at 14:21 +0100, Roel Kluin wrote:
>> With `while (stepcount-- > 0)' stepcount reaches -1 after the loop.
>
> This is true, but seems to be by design

>> - if (stepcount > 0 || Sgl_iszero(opnd1)) {
>> + if (stepcount >= 0 || Sgl_iszero(opnd1)) {

>
> Your patch does nothing to the actual execution flow (Sgl_iszero is true
> if stepcount == 0) ... what's the point of applying it?
>
> James

Ok, please ignore and thanks (also to Dave) for review.

Roel