Re: libgloss/arm/syscalls.c: missing BINARY mode when opening files

[Jeff Johnston <jjohnstn at redhat dot com>]

Christophe,

  O_BINARY is an exception to the rule as it is not part of the 
standards that newlib attempts to follow (e.g. POSIX, SUSV2, C89, C99).

  I don't think that ARM should be defining the other things that 
Windows/Cygwin defines (e.g. O_TEXT is not used or needed).  As well, I 
want to ween the shared header files of all the machine and sys 
exceptions in them and provide an easy way to override without having to 
copy all the current information over again so I am following what I did 
for machine/_types.h and have created an alternate patch.

  Basically, I have added a sys/fcntl.h to libc/sys/arm that overrides 
the current definition.  I have created a sys/_default_fcntl.h which is 
included by arm's sys/fcntl.h.  It is then the simple task of adding 
just the O_BINARY flag as needed.  Any other platform can do the same or 
add additional flags just as easily.  I eventually want to handle the 
Cygwin-only stuff the same way.

  Please try the accompanying patch.  If it works and there are no 
objections, I'll check it in.  The ChangeLog is below.

-- Jeff J.

2007-11-15  Jeff Johnston  <jjohnstn@redhat.com>

        * libc/include/sys/_default_fcntl.h: New header file that
        is the default version of sys/fcntl.h.
        * libc/include/sys/fcntl.h: Changed to simply include
        sys/_default_fcntl.h.
        * libc/sys/arm/sys/fcntl.h: New file that includes
        sys/_default_fcntl.h and defines O_BINARY.


Christophe LYON wrote:
> Hello,
>
> I have been experiencing issues when reading files in the following 
> configuration:
> - compilation with arm-none-eabi-gcc (ie GCC for ARM)
> - execution through RVDebug (ie ARM's debugger) under Windows.
>
> After some investigation, I have discovered that in Newlib's _swiopen() 
> (in libgloss/arm/syscalls.c), there are the following 4 lines:
> #ifdef O_BINARY
>   if (flags & O_BINARY)
>     aflags |= 1;
> #endif
>
> where O_BINARY is NOT defined when cross-compiling for ARM.
> Indeed, in newlib/libc/include/sys/fcntl.h, we can see:
> #if defined (_WIN32) || defined (__CYGWIN__)
> #define _FBINARY        0x10000
> #define _FTEXT          0x20000
> #define _FNOINHERIT     0x40000
>
> #define O_BINARY        _FBINARY
> .....
>
> The result is that when opening a file in binary mode, (eg "rb"), the 
> 'binary' flag is discarded before performing the semi-hosting call. 
> Then, rvdebug interprets this as an open in TEXT mode, which leads to 
> unpleasant results.
>
> Amusingly, executing from a Linux-hosted rvdebug seems to open in BINARY 
> mode in the same circumstances (which is not surprising...).
>
> So, there is a discrepancy on ARM's side, but I would also say there is 
> a bug on Newlib's side.
>
> I propose to add || defined (__arm__) in fcntl.h.
>
> What do you think ?
>
> Thanks,
>
> Christophe.

Index: libc/include/sys/_default_fcntl.h
===================================================================
RCS file: libc/include/sys/_default_fcntl.h
diff -N libc/include/sys/_default_fcntl.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ libc/include/sys/_default_fcntl.h	15 Nov 2007 19:31:49 -0000
@@ -0,0 +1,183 @@
+
+#ifndef	_SYS__DEFAULT_FCNTL_H_
+#ifdef __cplusplus
+extern "C" {
+#endif
+#define	_SYS__DEFAULT_FCNTL_H_
+#include <_ansi.h>
+#define	_FOPEN		(-1)	/* from sys/file.h, kernel use only */
+#define	_FREAD		0x0001	/* read enabled */
+#define	_FWRITE		0x0002	/* write enabled */
+#define	_FAPPEND	0x0008	/* append (writes guaranteed at the end) */
+#define	_FMARK		0x0010	/* internal; mark during gc() */
+#define	_FDEFER		0x0020	/* internal; defer for next gc pass */
+#define	_FASYNC		0x0040	/* signal pgrp when data ready */
+#define	_FSHLOCK	0x0080	/* BSD flock() shared lock present */
+#define	_FEXLOCK	0x0100	/* BSD flock() exclusive lock present */
+#define	_FCREAT		0x0200	/* open with file create */
+#define	_FTRUNC		0x0400	/* open with truncation */
+#define	_FEXCL		0x0800	/* error on open if file exists */
+#define	_FNBIO		0x1000	/* non blocking I/O (sys5 style) */
+#define	_FSYNC		0x2000	/* do all writes synchronously */
+#define	_FNONBLOCK	0x4000	/* non blocking I/O (POSIX style) */
+#define	_FNDELAY	_FNONBLOCK	/* non blocking I/O (4.2 style) */
+#define	_FNOCTTY	0x8000	/* don't assign a ctty on this open */
+
+#define	O_ACCMODE	(O_RDONLY|O_WRONLY|O_RDWR)
+
+/*
+ * Flag values for open(2) and fcntl(2)
+ * The kernel adds 1 to the open modes to turn it into some
+ * combination of FREAD and FWRITE.
+ */
+#define	O_RDONLY	0		/* +1 == FREAD */
+#define	O_WRONLY	1		/* +1 == FWRITE */
+#define	O_RDWR		2		/* +1 == FREAD|FWRITE */
+#define	O_APPEND	_FAPPEND
+#define	O_CREAT		_FCREAT
+#define	O_TRUNC		_FTRUNC
+#define	O_EXCL		_FEXCL
+#define O_SYNC		_FSYNC
+/*	O_NDELAY	_FNDELAY 	set in include/fcntl.h */
+/*	O_NDELAY	_FNBIO 		set in include/fcntl.h */
+#define	O_NONBLOCK	_FNONBLOCK
+#define	O_NOCTTY	_FNOCTTY
+/* For machines which care - */
+#if defined (_WIN32) || defined (__CYGWIN__)
+#define _FBINARY        0x10000
+#define _FTEXT          0x20000
+#define _FNOINHERIT	0x40000
+
+#define O_BINARY	_FBINARY
+#define O_TEXT		_FTEXT
+#define O_NOINHERIT	_FNOINHERIT
+
+/* The windows header files define versions with a leading underscore.  */
+#define _O_RDONLY	O_RDONLY
+#define _O_WRONLY	O_WRONLY
+#define _O_RDWR		O_RDWR
+#define _O_APPEND	O_APPEND
+#define _O_CREAT	O_CREAT
+#define _O_TRUNC	O_TRUNC
+#define _O_EXCL		O_EXCL
+#define _O_TEXT		O_TEXT
+#define _O_BINARY	O_BINARY
+#define _O_RAW		O_BINARY
+#define _O_NOINHERIT	O_NOINHERIT
+#endif
+
+#ifndef	_POSIX_SOURCE
+
+/*
+ * Flags that work for fcntl(fd, F_SETFL, FXXXX)
+ */
+#define	FAPPEND		_FAPPEND
+#define	FSYNC		_FSYNC
+#define	FASYNC		_FASYNC
+#define	FNBIO		_FNBIO
+#define	FNONBIO		_FNONBLOCK	/* XXX fix to be NONBLOCK everywhere */
+#define	FNDELAY		_FNDELAY
+
+/*
+ * Flags that are disallowed for fcntl's (FCNTLCANT);
+ * used for opens, internal state, or locking.
+ */
+#define	FREAD		_FREAD
+#define	FWRITE		_FWRITE
+#define	FMARK		_FMARK
+#define	FDEFER		_FDEFER
+#define	FSHLOCK		_FSHLOCK
+#define	FEXLOCK		_FEXLOCK
+
+/*
+ * The rest of the flags, used only for opens
+ */
+#define	FOPEN		_FOPEN
+#define	FCREAT		_FCREAT
+#define	FTRUNC		_FTRUNC
+#define	FEXCL		_FEXCL
+#define	FNOCTTY		_FNOCTTY
+
+#endif	/* !_POSIX_SOURCE */
+
+/* XXX close on exec request; must match UF_EXCLOSE in user.h */
+#define	FD_CLOEXEC	1	/* posix */
+
+/* fcntl(2) requests */
+#define	F_DUPFD		0	/* Duplicate fildes */
+#define	F_GETFD		1	/* Get fildes flags (close on exec) */
+#define	F_SETFD		2	/* Set fildes flags (close on exec) */
+#define	F_GETFL		3	/* Get file flags */
+#define	F_SETFL		4	/* Set file flags */
+#ifndef	_POSIX_SOURCE
+#define	F_GETOWN 	5	/* Get owner - for ASYNC */
+#define	F_SETOWN 	6	/* Set owner - for ASYNC */
+#endif	/* !_POSIX_SOURCE */
+#define	F_GETLK  	7	/* Get record-locking information */
+#define	F_SETLK  	8	/* Set or Clear a record-lock (Non-Blocking) */
+#define	F_SETLKW 	9	/* Set or Clear a record-lock (Blocking) */
+#ifndef	_POSIX_SOURCE
+#define	F_RGETLK 	10	/* Test a remote lock to see if it is blocked */
+#define	F_RSETLK 	11	/* Set or unlock a remote lock */
+#define	F_CNVT 		12	/* Convert a fhandle to an open fd */
+#define	F_RSETLKW 	13	/* Set or Clear remote record-lock(Blocking) */
+#endif	/* !_POSIX_SOURCE */
+
+/* fcntl(2) flags (l_type field of flock structure) */
+#define	F_RDLCK		1	/* read lock */
+#define	F_WRLCK		2	/* write lock */
+#define	F_UNLCK		3	/* remove lock(s) */
+#ifndef	_POSIX_SOURCE
+#define	F_UNLKSYS	4	/* remove remote locks for a given system */
+#endif	/* !_POSIX_SOURCE */
+
+/*#include <sys/stdtypes.h>*/
+
+#ifndef __CYGWIN__
+/* file segment locking set data type - information passed to system by user */
+struct flock {
+	short	l_type;		/* F_RDLCK, F_WRLCK, or F_UNLCK */
+	short	l_whence;	/* flag to choose starting offset */
+	long	l_start;	/* relative offset, in bytes */
+	long	l_len;		/* length, in bytes; 0 means lock to EOF */
+	short	l_pid;		/* returned with F_GETLK */
+	short	l_xxx;		/* reserved for future use */
+};
+#endif /* __CYGWIN__ */
+
+#ifndef	_POSIX_SOURCE
+/* extended file segment locking set data type */
+struct eflock {
+	short	l_type;		/* F_RDLCK, F_WRLCK, or F_UNLCK */
+	short	l_whence;	/* flag to choose starting offset */
+	long	l_start;	/* relative offset, in bytes */
+	long	l_len;		/* length, in bytes; 0 means lock to EOF */
+	short	l_pid;		/* returned with F_GETLK */
+	short	l_xxx;		/* reserved for future use */
+	long	l_rpid;		/* Remote process id wanting this lock */
+	long	l_rsys;		/* Remote system id wanting this lock */
+};
+#endif	/* !_POSIX_SOURCE */
+
+
+#include <sys/types.h>
+#include <sys/stat.h>		/* sigh. for the mode bits for open/creat */
+
+extern int open _PARAMS ((const char *, int, ...));
+extern int creat _PARAMS ((const char *, mode_t));
+extern int fcntl _PARAMS ((int, int, ...));
+
+/* Provide _<systemcall> prototypes for functions provided by some versions
+   of newlib.  */
+#ifdef _COMPILING_NEWLIB
+extern int _open _PARAMS ((const char *, int, ...));
+extern int _fcntl _PARAMS ((int, int, ...));
+#ifdef __LARGE64_FILES
+extern int _open64 _PARAMS ((const char *, int, ...));
+#endif
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+#endif	/* !_SYS__DEFAULT_FCNTL_H_ */
Index: libc/include/sys/fcntl.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/sys/fcntl.h,v
retrieving revision 1.6
diff -u -p -r1.6 fcntl.h
--- libc/include/sys/fcntl.h	14 Dec 2005 09:06:00 -0000	1.6
+++ libc/include/sys/fcntl.h	15 Nov 2007 19:31:49 -0000
@@ -1,183 +1,4 @@
-
-#ifndef	_FCNTL_
-#ifdef __cplusplus
-extern "C" {
+#ifndef _SYS_FCNTL_H_
+#define _SYS_FCNTL_H_
+#include <sys/_default_fcntl.h>
 #endif
-#define	_FCNTL_
-#include <_ansi.h>
-#define	_FOPEN		(-1)	/* from sys/file.h, kernel use only */
-#define	_FREAD		0x0001	/* read enabled */
-#define	_FWRITE		0x0002	/* write enabled */
-#define	_FAPPEND	0x0008	/* append (writes guaranteed at the end) */
-#define	_FMARK		0x0010	/* internal; mark during gc() */
-#define	_FDEFER		0x0020	/* internal; defer for next gc pass */
-#define	_FASYNC		0x0040	/* signal pgrp when data ready */
-#define	_FSHLOCK	0x0080	/* BSD flock() shared lock present */
-#define	_FEXLOCK	0x0100	/* BSD flock() exclusive lock present */
-#define	_FCREAT		0x0200	/* open with file create */
-#define	_FTRUNC		0x0400	/* open with truncation */
-#define	_FEXCL		0x0800	/* error on open if file exists */
-#define	_FNBIO		0x1000	/* non blocking I/O (sys5 style) */
-#define	_FSYNC		0x2000	/* do all writes synchronously */
-#define	_FNONBLOCK	0x4000	/* non blocking I/O (POSIX style) */
-#define	_FNDELAY	_FNONBLOCK	/* non blocking I/O (4.2 style) */
-#define	_FNOCTTY	0x8000	/* don't assign a ctty on this open */
-
-#define	O_ACCMODE	(O_RDONLY|O_WRONLY|O_RDWR)
-
-/*
- * Flag values for open(2) and fcntl(2)
- * The kernel adds 1 to the open modes to turn it into some
- * combination of FREAD and FWRITE.
- */
-#define	O_RDONLY	0		/* +1 == FREAD */
-#define	O_WRONLY	1		/* +1 == FWRITE */
-#define	O_RDWR		2		/* +1 == FREAD|FWRITE */
-#define	O_APPEND	_FAPPEND
-#define	O_CREAT		_FCREAT
-#define	O_TRUNC		_FTRUNC
-#define	O_EXCL		_FEXCL
-#define O_SYNC		_FSYNC
-/*	O_NDELAY	_FNDELAY 	set in include/fcntl.h */
-/*	O_NDELAY	_FNBIO 		set in include/fcntl.h */
-#define	O_NONBLOCK	_FNONBLOCK
-#define	O_NOCTTY	_FNOCTTY
-/* For machines which care - */
-#if defined (_WIN32) || defined (__CYGWIN__)
-#define _FBINARY        0x10000
-#define _FTEXT          0x20000
-#define _FNOINHERIT	0x40000
-
-#define O_BINARY	_FBINARY
-#define O_TEXT		_FTEXT
-#define O_NOINHERIT	_FNOINHERIT
-
-/* The windows header files define versions with a leading underscore.  */
-#define _O_RDONLY	O_RDONLY
-#define _O_WRONLY	O_WRONLY
-#define _O_RDWR		O_RDWR
-#define _O_APPEND	O_APPEND
-#define _O_CREAT	O_CREAT
-#define _O_TRUNC	O_TRUNC
-#define _O_EXCL		O_EXCL
-#define _O_TEXT		O_TEXT
-#define _O_BINARY	O_BINARY
-#define _O_RAW		O_BINARY
-#define _O_NOINHERIT	O_NOINHERIT
-#endif
-
-#ifndef	_POSIX_SOURCE
-
-/*
- * Flags that work for fcntl(fd, F_SETFL, FXXXX)
- */
-#define	FAPPEND		_FAPPEND
-#define	FSYNC		_FSYNC
-#define	FASYNC		_FASYNC
-#define	FNBIO		_FNBIO
-#define	FNONBIO		_FNONBLOCK	/* XXX fix to be NONBLOCK everywhere */
-#define	FNDELAY		_FNDELAY
-
-/*
- * Flags that are disallowed for fcntl's (FCNTLCANT);
- * used for opens, internal state, or locking.
- */
-#define	FREAD		_FREAD
-#define	FWRITE		_FWRITE
-#define	FMARK		_FMARK
-#define	FDEFER		_FDEFER
-#define	FSHLOCK		_FSHLOCK
-#define	FEXLOCK		_FEXLOCK
-
-/*
- * The rest of the flags, used only for opens
- */
-#define	FOPEN		_FOPEN
-#define	FCREAT		_FCREAT
-#define	FTRUNC		_FTRUNC
-#define	FEXCL		_FEXCL
-#define	FNOCTTY		_FNOCTTY
-
-#endif	/* !_POSIX_SOURCE */
-
-/* XXX close on exec request; must match UF_EXCLOSE in user.h */
-#define	FD_CLOEXEC	1	/* posix */
-
-/* fcntl(2) requests */
-#define	F_DUPFD		0	/* Duplicate fildes */
-#define	F_GETFD		1	/* Get fildes flags (close on exec) */
-#define	F_SETFD		2	/* Set fildes flags (close on exec) */
-#define	F_GETFL		3	/* Get file flags */
-#define	F_SETFL		4	/* Set file flags */
-#ifndef	_POSIX_SOURCE
-#define	F_GETOWN 	5	/* Get owner - for ASYNC */
-#define	F_SETOWN 	6	/* Set owner - for ASYNC */
-#endif	/* !_POSIX_SOURCE */
-#define	F_GETLK  	7	/* Get record-locking information */
-#define	F_SETLK  	8	/* Set or Clear a record-lock (Non-Blocking) */
-#define	F_SETLKW 	9	/* Set or Clear a record-lock (Blocking) */
-#ifndef	_POSIX_SOURCE
-#define	F_RGETLK 	10	/* Test a remote lock to see if it is blocked */
-#define	F_RSETLK 	11	/* Set or unlock a remote lock */
-#define	F_CNVT 		12	/* Convert a fhandle to an open fd */
-#define	F_RSETLKW 	13	/* Set or Clear remote record-lock(Blocking) */
-#endif	/* !_POSIX_SOURCE */
-
-/* fcntl(2) flags (l_type field of flock structure) */
-#define	F_RDLCK		1	/* read lock */
-#define	F_WRLCK		2	/* write lock */
-#define	F_UNLCK		3	/* remove lock(s) */
-#ifndef	_POSIX_SOURCE
-#define	F_UNLKSYS	4	/* remove remote locks for a given system */
-#endif	/* !_POSIX_SOURCE */
-
-/*#include <sys/stdtypes.h>*/
-
-#ifndef __CYGWIN__
-/* file segment locking set data type - information passed to system by user */
-struct flock {
-	short	l_type;		/* F_RDLCK, F_WRLCK, or F_UNLCK */
-	short	l_whence;	/* flag to choose starting offset */
-	long	l_start;	/* relative offset, in bytes */
-	long	l_len;		/* length, in bytes; 0 means lock to EOF */
-	short	l_pid;		/* returned with F_GETLK */
-	short	l_xxx;		/* reserved for future use */
-};
-#endif /* __CYGWIN__ */
-
-#ifndef	_POSIX_SOURCE
-/* extended file segment locking set data type */
-struct eflock {
-	short	l_type;		/* F_RDLCK, F_WRLCK, or F_UNLCK */
-	short	l_whence;	/* flag to choose starting offset */
-	long	l_start;	/* relative offset, in bytes */
-	long	l_len;		/* length, in bytes; 0 means lock to EOF */
-	short	l_pid;		/* returned with F_GETLK */
-	short	l_xxx;		/* reserved for future use */
-	long	l_rpid;		/* Remote process id wanting this lock */
-	long	l_rsys;		/* Remote system id wanting this lock */
-};
-#endif	/* !_POSIX_SOURCE */
-
-
-#include <sys/types.h>
-#include <sys/stat.h>		/* sigh. for the mode bits for open/creat */
-
-extern int open _PARAMS ((const char *, int, ...));
-extern int creat _PARAMS ((const char *, mode_t));
-extern int fcntl _PARAMS ((int, int, ...));
-
-/* Provide _<systemcall> prototypes for functions provided by some versions
-   of newlib.  */
-#ifdef _COMPILING_NEWLIB
-extern int _open _PARAMS ((const char *, int, ...));
-extern int _fcntl _PARAMS ((int, int, ...));
-#ifdef __LARGE64_FILES
-extern int _open64 _PARAMS ((const char *, int, ...));
-#endif
-#endif
-
-#ifdef __cplusplus
-}
-#endif
-#endif	/* !_FCNTL_ */
Index: libc/sys/arm/sys/fcntl.h
===================================================================
RCS file: libc/sys/arm/sys/fcntl.h
diff -N libc/sys/arm/sys/fcntl.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ libc/sys/arm/sys/fcntl.h	15 Nov 2007 19:31:49 -0000
@@ -0,0 +1,12 @@
+#ifndef _SYS_FCNTL_H_
+#define _SYS_FCNTL_H_
+
+#include <sys/_default_fcntl.h>
+
+/* We want to support O_BINARY for the open syscall.
+   For example, the Demon debug monitor has a separate
+   flag value for "rb" vs "r". */
+#define _FBINARY        0x10000
+#define O_BINARY        _FBINARY
+
+#endif /* _SYS_FCNTL_H_ */