On Tue, Jul 14, 2009 at 12:03:32PM +1000, Geoff Wing wrote:
> On Tuesday 2009-07-14 11:50 +1000, matthew green output:
> : - newlen = off + len + 1;
> : /* Ensure that the resultant buffer length fits in ssize_t */
> : - if (newlen > (size_t)SSIZE_MAX + 1) {
> : + if (off + len + 1 > (unsigned int)SSIZE_MAX) {
>
> :unsigned int will truncate this on 64 bit platforms, won't it?
> :can't the cast just be removed?
>
> I guess so. I don't remember how the compiler chooses the comparison
> for this. "off", "len" and "SSIZE_MAX" are all ``ssize_t''. I would
> have thought you needed to expand one of them at least (even if, as you
> say, ``unsigned int'' is the wrong choice here) to get a correct
> comparison generated but I guess my C skills are a bit rusty.
>
> Regards,
> Geoff
Assuming "sizeof(size_t) >= sizeof(int)" on all arches the 3rd argument
to memchr() may be casted to size_t.
The expression "off + len + 1 > SIZE_T_MAX" may be written as
"len >= SIZE_T_MAX-1 || off > SIZE_T_MAX-len-1" without overflow.
Why must the length fit in ssize_t where all vars are size_t?
With these changes getdelim.c passes lint and compiles on i386 and sparc64:
Index: getdelim.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdio/getdelim.c,v
retrieving revision 1.1
diff -p -u -2 -r1.1 getdelim.c
--- getdelim.c 13 Jul 2009 22:19:25 -0000 1.1
+++ getdelim.c 14 Jul 2009 15:42:05 -0000
@@ -80,5 +80,5 @@ getdelim(char **__restrict buf, size_t *
/* Scan through looking for the separator */
- p = memchr(fp->_p, sep, fp->_r);
+ p = memchr(fp->_p, sep, (size_t)fp->_r);
if (p == NULL)
len = fp->_r;
@@ -86,10 +86,10 @@ getdelim(char **__restrict buf, size_t *
len = (p - fp->_p) + 1;
- newlen = off + len + 1;
/* Ensure that the resultant buffer length fits in ssize_t */
- if (newlen > (size_t)SSIZE_MAX + 1) {
+ if (len >= SIZE_T_MAX-1 || off > SIZE_T_MAX-len-1) {
errno = EOVERFLOW;
goto error;
}
+ newlen = off + len + 1;
if (newlen > *buflen) {
if (newlen < MINBUF)
--
Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
