X-Authentication-Warning: acp3bf.physik.rwth-aachen.de: broeker owned process doing -bs Date: Thu, 1 Mar 2001 14:36:30 +0100 (MET) From: Hans-Bernhard Broeker X-Sender: broeker AT acp3bf To: djgpp workers list Subject: Re: strtol fix correct? In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Reply-To: djgpp-workers AT delorie DOT com Errors-To: nobody AT delorie DOT com X-Mailing-List: djgpp-workers AT delorie DOT com X-Unsubscribes-To: listserv AT delorie DOT com Precedence: bulk On Thu, 1 Mar 2001, Eli Zaretskii wrote: > On Thu, 1 Mar 2001, Hans-Bernhard Broeker wrote: > > Or at least replace those "& 0xff" operations by what they *really* > > do: cast to unsigned char, i.e. I'd like to propose to change that > > patched line and similar places to: > > > > unsigned char c; > > /* ... */ > > c = (unsigned char) *s++ > > This doesn't really work, unfortunately, at least not in all cases, due > to C promotion rules. The code in point cannot declare c unsigned char > (it's an int), and without that casting doesn't do any good. A little misunderstanding, there. I had meant to replace the existing declaration of 'c' by unsigned char. > > I haven't check whether this triggers signedness warnings, but if it does, > > then other, similar methods could and should be found that don't. > > Alas, that usually means almost complete rewrite of the code. I don't think so. The problem at hand arose because one execution path was overlooked by whoever did those late-bug-fix changes, last time, and could have been avoided by rigorously replacing every single instance of "c = *s++;" by "c = (*s++) & 0xff;". Changing s to unsigned char *, too, would reduce the number of places that have to be changed to a single one: the initialization of 's'. All the rest of strtol() could be left as is, and the 0xff stuff be removed. > If someone cares enough, they are welcome to do that, now that we > don't have to rush into a long-overdue bug-fix release. Well, I do care, so here's the diff I would propose. A test compile with my old GCC-2.8.1 cross-compiler on an Alpha/Tru64 box had no complaints. Index: strtol.c =================================================================== RCS file: /cvs/djgpp/djgpp/src/libc/ansi/stdlib/strtol.c,v retrieving revision 1.4 diff -u -p -3 -r1.4 strtol.c --- strtol.c 2001/02/28 20:55:12 1.4 +++ strtol.c 2001/03/01 13:34:40 @@ -9,9 +9,9 @@ long strtol(const char *nptr, char **endptr, int base) { - const char *s = nptr; + const unsigned char *s = (const unsigned char *) nptr; unsigned long acc; - int c; + unsigned char c; unsigned long cutoff; int neg = 0, any, cutlim; @@ -22,7 +22,7 @@ strtol(const char *nptr, char **endptr, */ do { c = *s++; - } while (isspace(c & 0xff)); + } while (isspace(c)); if (c == '-') { neg = 1; @@ -60,7 +60,7 @@ strtol(const char *nptr, char **endptr, cutoff = neg ? -(unsigned long)LONG_MIN : LONG_MAX; cutlim = cutoff % (unsigned long)base; cutoff /= (unsigned long)base; - for (acc = 0, any = 0, c &= 0xff;; c = *s++, c &= 0xff) + for (acc = 0, any = 0; ; c = *s++) { if (isdigit(c)) c -= '0'; -- Hans-Bernhard Broeker (broeker AT physik DOT rwth-aachen DOT de) Even if all the snow were burnt, ashes would remain.