Re: CVS commit: src (getdelim.c:1.1)

[Juergen Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost>]

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 - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)