bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [coreutils] coreutils-7.0 expr exposes long-standing bug in matlab s


From: Pádraig Brady
Subject: Re: [coreutils] coreutils-7.0 expr exposes long-standing bug in matlab startup script
Date: Fri, 17 Oct 2008 00:04:24 +0100
User-agent: Thunderbird 2.0.0.6 (X11/20071008)

Excellent stuff. You got a net reduction of 366 lines,
and another 91 lines are not used in the common case
where GMP is available.

I'm a bit worried by signed overflow checking in the following.
I know gcc won't do it's ignore undefined signed overflow
code shenanigans with this, but does this assume 2s compliment
architectures? If so is this OK?

+static void
+mpz_add (mpz_t r, mpz_t a0, mpz_t b0)
+{
+  intmax_t a = a0[0];
+  intmax_t b = b0[0];
+  intmax_t val = a + b;
+  if ((val < a) != (b < 0))
+    integer_overflow ('+');

What about the INT_MIN/-1 exception case in the following.
Oh I see it's short circuited in that case.
This code deserves an award for brevity or something :)

+mpz_mul (mpz_t r, mpz_t a0, mpz_t b0)
+{
+  intmax_t a = a0[0];
+  intmax_t b = b0[0];
+  intmax_t val = a * b;
+  if (! (a == 0 || b == 0
+        || ((val < 0) == ((a < 0) ^ (b < 0)) && val / a == b)))
+    integer_overflow ('*');

I've attached a patch to standardize the format of the
AUTHORS define to that used in other utils with multiple authors.
I've also added you to the list since you basically rewrote
expr with this patch.

cheers,
Pádraig.
>From 057e83d69a6b212e6aa3c0943065609ab06877c5 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 16 Oct 2008 19:09:59 +0100
Subject: [PATCH] expr: Fixup authors

* src/expr.c: Standardise the format of AUTHORS to
that used in other utils with multiple authors.
Also add Paul Eggert since he basically rewrote it
with his bignum fixes.
* AUTHORS (expr): Add Paul Eggert.
---
 AUTHORS    |    2 +-
 src/expr.c |    5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 9c3d990..fa3c029 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -25,7 +25,7 @@ du: Torbjörn Granlund, David MacKenzie, Paul Eggert, Jim 
Meyering
 echo: Brian Fox, Chet Ramey
 env: Richard Mlynarik, David MacKenzie
 expand: David MacKenzie
-expr: Mike Parker, James Youngman
+expr: Mike Parker, James Youngman, Paul Eggert
 factor: Paul Rubin
 false: Jim Meyering
 fmt: Ross Paterson
diff --git a/src/expr.c b/src/expr.c
index de7a63d..65e74af 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -145,7 +145,10 @@ mpz_out_str (FILE *stream, int base, mpz_t z)
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "expr"
 
-#define AUTHORS proper_name ("Mike Parker"), proper_name ("James Youngman")
+#define AUTHORS \
+  proper_name ("Mike Parker"), \
+  proper_name ("James Youngman"), \
+  proper_name ("Paul Eggert")
 
 /* Exit statuses.  */
 enum
-- 
1.5.3.6


reply via email to

[Prev in Thread] Current Thread [Next in Thread]