Re: Reading a write-only file doesn't set error condition (was Re: Cygwin fread on Write-Only File Descriptor returns undefined state)

[Corinna Vinschen <vinschen at redhat dot com>]

On Jun 14 16:57, Jeff Johnston wrote:
> Jeff Johnston wrote:
> >Corinna Vinschen wrote:
> >>So, I'd like to propose the below patch.  I assume a similar patch
> >>should be ok for __sfvwrite, too, isn't it?  There's a call to
> >>cantwrite() which only returns EOF but which probably should also
> >>set the error condition and errno.
> >>
> >
> >This opens a can of worms.  You can't just use _REENT to set errno.  One 
> >has to provide _r versions of the read/write functions.  I have just 
> >made a patch and am verifying it builds for x86-linux.  I will post the 
> >patch when I am ready to check it in.
> >
> >-- Jeff J.
> >
> 
> I have checked in the patch.  The testcase runs successfully and I am 
> able to build/run x86-linux successfully.  Let me know if it causes any 
> problems for Cygwin.

Thanks for the patch, but unfortunately it does cause problems.
I found three problems:

- Typo in stdio.h, "struct _REENT" instead of "struct _reent".
- Missing conversion from crlf to crlf_r calls in fread.c.
- Removing __srget and __swbuf functions breaks backward compatibility.
  Older applications built up to yesterday against a share lib version
  of newlib will refuse to run with a newer newlib, due to missing
  symbols __srget and __swbuf in the shared lib.

Patch below.


Thanks,
Corinna


	* libc/include/stdio.h (__sgetc_r): Fix typo.
	* libc/stdio/fread.c (_fread_r): Convert crlf calls to crlf_r.
	* libc/stdio/rget.c (__srbuf): Reinstantiate.
	* libc/stdio/wbuf.c (__swbuf): Ditto.


Index: libc/include/stdio.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/stdio.h,v
retrieving revision 1.36
diff -u -p -r1.36 stdio.h
--- libc/include/stdio.h	14 Jun 2006 20:49:11 -0000	1.36
+++ libc/include/stdio.h	15 Jun 2006 12:28:24 -0000
@@ -402,7 +402,7 @@ FILE	*_EXFUN(funopen,(const _PTR _cookie
 #define       __sgetc_raw_r(__ptr, __f) (--(__f)->_r < 0 ? __srget_r(__ptr, __f) : (int)(*(__f)->_p++))
 
 #ifdef __SCLE
-static __inline__ int __sgetc_r(struct _REENT *__ptr, FILE *__p)
+static __inline__ int __sgetc_r(struct _reent *__ptr, FILE *__p)
   {
     int __c = __sgetc_raw_r(__ptr, __p);
     if ((__p->_flags & __SCLE) && (__c == '\r'))
Index: libc/stdio/fread.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fread.c,v
retrieving revision 1.11
diff -u -p -r1.11 fread.c
--- libc/stdio/fread.c	14 Jun 2006 20:49:11 -0000	1.11
+++ libc/stdio/fread.c	15 Jun 2006 12:28:24 -0000
@@ -196,7 +196,7 @@ _DEFUN(_fread_r, (ptr, buf, size, count,
               if (fp->_flags & __SCLE)
 	        {
 	          _funlockfile (fp);
-	          return crlf (fp, buf, total-resid, 1) / size;
+	          return crlf_r (ptr, fp, buf, total-resid, 1) / size;
 	        }
 #endif
 	      _funlockfile (fp);
@@ -221,7 +221,7 @@ _DEFUN(_fread_r, (ptr, buf, size, count,
 	      if (fp->_flags & __SCLE)
 		{
 		  _funlockfile (fp);
-		  return crlf (fp, buf, total-resid, 1) / size;
+		  return crlf_r (ptr, fp, buf, total-resid, 1) / size;
 		}
 #endif
 	      _funlockfile (fp);
@@ -238,7 +238,7 @@ _DEFUN(_fread_r, (ptr, buf, size, count,
   if (fp->_flags & __SCLE)
     {
       _funlockfile (fp);
-      return crlf(fp, buf, total, 0) / size;
+      return crlf_r(ptr, fp, buf, total, 0) / size;
     }
 #endif
   _funlockfile (fp);
Index: libc/stdio/rget.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/rget.c,v
retrieving revision 1.3
diff -u -p -r1.3 rget.c
--- libc/stdio/rget.c	14 Jun 2006 20:49:11 -0000	1.3
+++ libc/stdio/rget.c	15 Jun 2006 12:28:24 -0000
@@ -43,3 +43,13 @@ _DEFUN(__srget_r, (ptr, fp),
     }
   return EOF;
 }
+
+/* This function isn't any longer declared in stdio.h, but it's
+   required for backward compatibility with applications built against
+   earlier dynamically built newlib libraries. */
+int
+_DEFUN(__srget, (fp),
+       register FILE *fp)
+{
+  return __srget_r (_REENT, fp);
+}
Index: libc/stdio/wbuf.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/wbuf.c,v
retrieving revision 1.4
diff -u -p -r1.4 wbuf.c
--- libc/stdio/wbuf.c	14 Jun 2006 20:49:11 -0000	1.4
+++ libc/stdio/wbuf.c	15 Jun 2006 12:28:24 -0000
@@ -85,3 +85,14 @@ _DEFUN(__swbuf_r, (ptr, c, fp),
       return EOF;
   return c;
 }
+
+/* This function isn't any longer declared in stdio.h, but it's
+   required for backward compatibility with applications built against
+   earlier dynamically built newlib libraries. */
+int
+_DEFUN(__swbuf, (c, fp),
+       register int c _AND
+       register FILE *fp)
+{
+  return __swbuf_r (_REENT, c, fp);
+}

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat