fix null pointer subtraction and comparison in stdio
authorRich Felker <dalias@aerifal.cx>
Sun, 16 Sep 2018 17:46:46 +0000 (13:46 -0400)
committerRich Felker <dalias@aerifal.cx>
Sun, 16 Sep 2018 18:37:22 +0000 (14:37 -0400)
morally, for null pointers a and b, a-b, a<b, and a>b should all be
defined as 0; however, C does not define any of them.

the stdio implementation makes heavy use of such pointer comparison
and subtraction for buffer logic, and also uses null pos/base/end
pointers to indicate that the FILE is not in the corresponding (read
or write) mode ready for accesses through the buffer.

all of the comparisons are fixed trivially by using != in place of the
relational operators, since the opposite relation (e.g. pos>end) is
logically impossible. the subtractions have been reviewed to check
that they are conditional the stream being in the appropriate reading-
or writing-through-buffer mode, with checks added where needed.

in fgets and getdelim, the checks added should improve performance for
unbuffered streams by avoiding a do-nothing call to memchr, and should
be negligible for buffered streams.

14 files changed:
src/internal/stdio_impl.h
src/stdio/__overflow.c
src/stdio/__stdio_exit.c
src/stdio/__toread.c
src/stdio/ext2.c
src/stdio/fflush.c
src/stdio/fgetln.c
src/stdio/fgets.c
src/stdio/fgetwc.c
src/stdio/fread.c
src/stdio/fseek.c
src/stdio/ftell.c
src/stdio/getdelim.c
src/stdio/vfwscanf.c

index 4afb7ea25036ed4ea26cfacfce8177e4d66430d7..8c81fd53c6585a4946887ed73003bb0f0e5b15b5 100644 (file)
@@ -100,10 +100,10 @@ hidden void __getopt_msg(const char *, const char *, const char *, size_t);
 #define ferror(f) ((f)->flags & F_ERR)
 
 #define getc_unlocked(f) \
-       ( ((f)->rpos < (f)->rend) ? *(f)->rpos++ : __uflow((f)) )
+       ( ((f)->rpos != (f)->rend) ? *(f)->rpos++ : __uflow((f)) )
 
 #define putc_unlocked(c, f) \
-       ( ((unsigned char)(c)!=(f)->lbf && (f)->wpos<(f)->wend) \
+       ( ((unsigned char)(c)!=(f)->lbf && (f)->wpos!=(f)->wend) \
        ? *(f)->wpos++ = (c) : __overflow((f),(c)) )
 
 /* Caller-allocated FILE * operations */
index 3bb37923abb847787878cf868d86bce570fb267e..e65a594dd7bf190a5481fc707a8f902812fe0ca3 100644 (file)
@@ -4,7 +4,7 @@ int __overflow(FILE *f, int _c)
 {
        unsigned char c = _c;
        if (!f->wend && __towrite(f)) return EOF;
-       if (f->wpos < f->wend && c != f->lbf) return *f->wpos++ = c;
+       if (f->wpos != f->wend && c != f->lbf) return *f->wpos++ = c;
        if (f->write(f, &c, 1)!=1) return EOF;
        return c;
 }
index 5741070f79118c42eced380bff2ce09b877d1071..a5e42c675fa90c4edc2ebde2576aaa543ed91a14 100644 (file)
@@ -9,8 +9,8 @@ static void close_file(FILE *f)
 {
        if (!f) return;
        FFINALLOCK(f);
-       if (f->wpos > f->wbase) f->write(f, 0, 0);
-       if (f->rpos < f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
+       if (f->wpos != f->wbase) f->write(f, 0, 0);
+       if (f->rpos != f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
 }
 
 void __stdio_exit(void)
index 309ee6e8961e4d0bd43b141cef9a89a105531369..f142ff09c087ef5459a29acd004fd005ddfc8a0c 100644 (file)
@@ -3,7 +3,7 @@
 int __toread(FILE *f)
 {
        f->mode |= f->mode-1;
-       if (f->wpos > f->wbase) f->write(f, 0, 0);
+       if (f->wpos != f->wbase) f->write(f, 0, 0);
        f->wpos = f->wbase = f->wend = 0;
        if (f->flags & F_NORD) {
                f->flags |= F_ERR;
index afd8b34e6ea6446db9a239f7ed0cdb4c4c6ba3a8..34162780131849adc93b4508516e0a4450bc07ed 100644 (file)
@@ -3,14 +3,13 @@
 
 size_t __freadahead(FILE *f)
 {
-       return f->rend - f->rpos;
+       return f->rend ? f->rend - f->rpos : 0;
 }
 
 const char *__freadptr(FILE *f, size_t *sizep)
 {
-       size_t size = f->rend - f->rpos;
-       if (!size) return 0;
-       *sizep = size;
+       if (f->rpos == f->rend) return 0;
+       *sizep = f->rend - f->rpos;
        return (const char *)f->rpos;
 }
 
index bf1e843765761a4566ccb15130c94a0cc449a2c5..02dae27a97a4a5281610741e6a510eec4e16fdd1 100644 (file)
@@ -11,7 +11,7 @@ int fflush(FILE *f)
 
                for (f=*__ofl_lock(); f; f=f->next) {
                        FLOCK(f);
-                       if (f->wpos > f->wbase) r |= fflush(f);
+                       if (f->wpos != f->wbase) r |= fflush(f);
                        FUNLOCK(f);
                }
                __ofl_unlock();
@@ -22,7 +22,7 @@ int fflush(FILE *f)
        FLOCK(f);
 
        /* If writing, flush output */
-       if (f->wpos > f->wbase) {
+       if (f->wpos != f->wbase) {
                f->write(f, 0, 0);
                if (!f->wpos) {
                        FUNLOCK(f);
@@ -31,7 +31,7 @@ int fflush(FILE *f)
        }
 
        /* If reading, sync position, per POSIX */
-       if (f->rpos < f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
+       if (f->rpos != f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
 
        /* Clear read and write modes */
        f->wpos = f->wbase = f->wend = 0;
index afe12b5dfe024add1739f80d5982eefef76d7567..5748435d9f6f937c07cd448182df0cc875f0e871 100644 (file)
@@ -8,7 +8,7 @@ char *fgetln(FILE *f, size_t *plen)
        ssize_t l;
        FLOCK(f);
        ungetc(getc_unlocked(f), f);
-       if ((z=memchr(f->rpos, '\n', f->rend - f->rpos))) {
+       if (f->rend && (z=memchr(f->rpos, '\n', f->rend - f->rpos))) {
                ret = (char *)f->rpos;
                *plen = ++z - ret;
                f->rpos = (void *)z;
index d3f9819e82eec3a5707805d5ae5b661a78adec30..6171f398dacb4170a5a1227d515788d5457b252e 100644 (file)
@@ -21,14 +21,16 @@ char *fgets(char *restrict s, int n, FILE *restrict f)
        }
 
        while (n) {
-               z = memchr(f->rpos, '\n', f->rend - f->rpos);
-               k = z ? z - f->rpos + 1 : f->rend - f->rpos;
-               k = MIN(k, n);
-               memcpy(p, f->rpos, k);
-               f->rpos += k;
-               p += k;
-               n -= k;
-               if (z || !n) break;
+               if (f->rpos != f->rend) {
+                       z = memchr(f->rpos, '\n', f->rend - f->rpos);
+                       k = z ? z - f->rpos + 1 : f->rend - f->rpos;
+                       k = MIN(k, n);
+                       memcpy(p, f->rpos, k);
+                       f->rpos += k;
+                       p += k;
+                       n -= k;
+                       if (z || !n) break;
+               }
                if ((c = getc_unlocked(f)) < 0) {
                        if (p==s || !feof(f)) s = 0;
                        break;
index 07fb6d7c294fe2c55a7e39ff48c1451c8514150a..0801e28f9de1d5c70bc3c9850bb181e4e395a6d1 100644 (file)
@@ -10,7 +10,7 @@ static wint_t __fgetwc_unlocked_internal(FILE *f)
        size_t l;
 
        /* Convert character from buffer if possible */
-       if (f->rpos < f->rend) {
+       if (f->rpos != f->rend) {
                l = mbtowc(&wc, (void *)f->rpos, f->rend - f->rpos);
                if (l+1 >= 1) {
                        f->rpos += l + !l; /* l==0 means 1 byte, null */
index 733d37163aacf0ce20d2585dab0694c513db499a..a2116da61370bd829c634353ad6f6fa6732d0d8b 100644 (file)
@@ -13,7 +13,7 @@ size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
 
        f->mode |= f->mode-1;
 
-       if (f->rend - f->rpos > 0) {
+       if (f->rpos != f->rend) {
                /* First exhaust the buffer. */
                k = MIN(f->rend - f->rpos, l);
                memcpy(dest, f->rpos, k);
index 67d75f7a7627a5e870a57ea5dfc59bc7d7b30115..439308f7574ba7efbd03ea9626c9041f3b7a322c 100644 (file)
@@ -3,10 +3,10 @@
 int __fseeko_unlocked(FILE *f, off_t off, int whence)
 {
        /* Adjust relative offset for unread data in buffer, if any. */
-       if (whence == SEEK_CUR) off -= f->rend - f->rpos;
+       if (whence == SEEK_CUR && f->rend) off -= f->rend - f->rpos;
 
        /* Flush write buffer, and report error on failure. */
-       if (f->wpos > f->wbase) {
+       if (f->wpos != f->wbase) {
                f->write(f, 0, 0);
                if (!f->wpos) return -1;
        }
index 5ca416549dd901cb8ced1160c276136066b09987..1a2afbbce3a62ef87f90c7093d11bd8f5369da84 100644 (file)
@@ -5,12 +5,16 @@
 off_t __ftello_unlocked(FILE *f)
 {
        off_t pos = f->seek(f, 0,
-               (f->flags & F_APP) && f->wpos > f->wbase
+               (f->flags & F_APP) && f->wpos != f->wbase
                ? SEEK_END : SEEK_CUR);
        if (pos < 0) return pos;
 
        /* Adjust for data in buffer. */
-       return pos - (f->rend - f->rpos) + (f->wpos - f->wbase);
+       if (f->rend)
+               pos += f->rpos - f->rend;
+       else if (f->wbase)
+               pos += f->wpos - f->wbase;
+       return pos;
 }
 
 off_t __ftello(FILE *f)
index 60c6cc18677380a3e8c63fbe9439c20a235a6b32..c313775d3d026f11a9660e21b888959ea35f680a 100644 (file)
@@ -25,8 +25,13 @@ ssize_t getdelim(char **restrict s, size_t *restrict n, int delim, FILE *restric
        if (!*s) *n=0;
 
        for (;;) {
-               z = memchr(f->rpos, delim, f->rend - f->rpos);
-               k = z ? z - f->rpos + 1 : f->rend - f->rpos;
+               if (f->rpos != f->rend) {
+                       z = memchr(f->rpos, delim, f->rend - f->rpos);
+                       k = z ? z - f->rpos + 1 : f->rend - f->rpos;
+               } else {
+                       z = 0;
+                       k = 0;
+               }
                if (i+k+1 >= *n) {
                        if (k >= SIZE_MAX/2-i) goto oom;
                        size_t m = i+k+2;
index 7be663440bcbd250c401f23c0cfd692f66ad154c..82f4860441e5b41e52d2e27ddefb2fa1b50ecafb 100644 (file)
@@ -76,7 +76,7 @@ static int in_set(const wchar_t *set, int c)
 #if 1
 #undef getwc
 #define getwc(f) \
-       ((f)->rpos < (f)->rend && *(f)->rpos < 128 ? *(f)->rpos++ : (getwc)(f))
+       ((f)->rpos != (f)->rend && *(f)->rpos < 128 ? *(f)->rpos++ : (getwc)(f))
 
 #undef ungetwc
 #define ungetwc(c,f) \