Re: newlib and the use of fcntl by java

["J. Johnston" <jjohnstn at redhat dot com>]

Richard Earnshaw wrote:
> 
> > On Tue, 2002-09-17 at 06:38, Richard Earnshaw wrote:
> > > Hmm, it appears that java is making use of the fcntl function in newlib
> > > (or at least, it is trying to).  This is a problem, since most embedded
> > > targets don't provide this function.  The function fcntl always exists in
> > > newlib's libc.a, but on most targets it will call a non-existent function
> > > _fcntl which causes a link error.
> >
> > I have a patch which adds _fcntl for the ARM newlib port.  It was even
> > approved, but, being the bad hacker-citizen that I am, it was never
> > committed.
> >
> > http://sources.redhat.com/ml/newlib/2002/msg00105.html
> >
> > However, I'll note that adding a dummy _fcntl (like in my patch) wasn't
> > one of your suggested fixes.  I believe syscalls contains other dummy
> > implementations - so it seemed like the thing to do.  What do you think?
> >
> > Thanks,
> >
> > AG
> >
> >
> 
> The problem I see with providing a default that doesn't do the right thing
> (TM) is that an application can then get confused, since it made the call
> but the effects didn't happen as expected.  If we can't do what's
> required, I'm not sure we should quietly ignore things.  (Even setting
> errno and returning -1 might not be enough.)
> 
> R.

There are only a few places in newlib-generic that call fcntl.  One is already protected
with the HAVE_FCNTL flag.  To be consistent, all should check the flag.  This
includes the code in the libc/syscalls directory.  As well, _fcntl_r needs to be
added to the libc/reent directory.

I have put together the attached patch which should cure the problems.  Any
platform that has fcntl should set the HAVE_FCNTL flag in configure.host.

2002-09-17  Jeff Johnston  <jjohnstn@redhat.com>

	* libc/reent/fcntlr.c: New file.
	* libc/reent/Makefile.am: Add fcntlr.c.
	* libc/reent/Makefile.in: Regenerated.
	* libc/syscalls/sysfcntl.c: Add check for HAVE_FCNTL flag.  Perform default
	stub if flag not set.
	* libc/stdio/fdopen.c (_fdopen_r): Call _fcntl_r instead of _fcntl.
	* libc/posix/opendir.c (opendir): Add check for HAVE_FCNTL before calling
	fcntl.
	* libc/search/hash.c: Ditto.
	* libc/search/hash_page.c: Ditto.


-- Jeff J.
Index: libc/posix/opendir.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/posix/opendir.c,v
retrieving revision 1.2
diff -u -r1.2 opendir.c
--- libc/posix/opendir.c	5 Jun 2002 20:58:56 -0000	1.2
+++ libc/posix/opendir.c	17 Sep 2002 19:50:27 -0000
@@ -52,10 +52,14 @@
 {
 	register DIR *dirp;
 	register int fd;
+	int rc = 0;
 
 	if ((fd = open(name, 0)) == -1)
 		return NULL;
-	if (fcntl(fd, F_SETFD, 1) == -1 ||
+#ifdef HAVE_FCNTL
+	rc = fcntl(fd, F_SETFD, 1);
+#endif
+	if (rc == -1 ||
 	    (dirp = (DIR *)malloc(sizeof(DIR))) == NULL) {
 		close (fd);
 		return NULL;
Index: libc/reent/Makefile.am
===================================================================
RCS file: /cvs/src/src/newlib/libc/reent/Makefile.am,v
retrieving revision 1.7
diff -u -r1.7 Makefile.am
--- libc/reent/Makefile.am	26 Aug 2002 18:56:06 -0000	1.7
+++ libc/reent/Makefile.am	17 Sep 2002 19:50:27 -0000
@@ -33,6 +33,7 @@
 	closer.c \
 	reent.c \
 	impure.c \
+	fcntlr.c \
 	fstatr.c \
 	getreent.c \
 	linkr.c \
@@ -69,6 +70,7 @@
 	closer.def \
 	reent.def \
 	execr.def \
+	fcntlr.def \
 	fstatr.def \
 	linkr.def \
 	lseekr.def \
Index: libc/reent/Makefile.in
===================================================================
RCS file: /cvs/src/src/newlib/libc/reent/Makefile.in,v
retrieving revision 1.10
diff -u -r1.10 Makefile.in
--- libc/reent/Makefile.in	26 Aug 2002 18:56:06 -0000	1.10
+++ libc/reent/Makefile.in	17 Sep 2002 19:50:27 -0000
@@ -130,6 +130,7 @@
 	closer.c \
 	reent.c \
 	impure.c \
+	fcntlr.c \
 	fstatr.c \
 	getreent.c \
 	linkr.c \
@@ -162,6 +163,7 @@
 	closer.def \
 	reent.def \
 	execr.def \
+	fcntlr.def \
 	fstatr.def \
 	linkr.def \
 	lseekr.def \
@@ -192,17 +194,17 @@
 CPPFLAGS = @CPPFLAGS@
 LIBS = @LIBS@
 @USE_LIBTOOL_FALSE@lib_a_OBJECTS =  closer.$(OBJEXT) reent.$(OBJEXT) \
-@USE_LIBTOOL_FALSE@impure.$(OBJEXT) fstatr.$(OBJEXT) getreent.$(OBJEXT) \
-@USE_LIBTOOL_FALSE@linkr.$(OBJEXT) lseekr.$(OBJEXT) openr.$(OBJEXT) \
-@USE_LIBTOOL_FALSE@readr.$(OBJEXT) signalr.$(OBJEXT) signgam.$(OBJEXT) \
-@USE_LIBTOOL_FALSE@sbrkr.$(OBJEXT) statr.$(OBJEXT) timer.$(OBJEXT) \
-@USE_LIBTOOL_FALSE@unlinkr.$(OBJEXT) writer.$(OBJEXT)
+@USE_LIBTOOL_FALSE@impure.$(OBJEXT) fcntlr.$(OBJEXT) fstatr.$(OBJEXT) \
+@USE_LIBTOOL_FALSE@getreent.$(OBJEXT) linkr.$(OBJEXT) lseekr.$(OBJEXT) \
+@USE_LIBTOOL_FALSE@openr.$(OBJEXT) readr.$(OBJEXT) signalr.$(OBJEXT) \
+@USE_LIBTOOL_FALSE@signgam.$(OBJEXT) sbrkr.$(OBJEXT) statr.$(OBJEXT) \
+@USE_LIBTOOL_FALSE@timer.$(OBJEXT) unlinkr.$(OBJEXT) writer.$(OBJEXT)
 LTLIBRARIES =  $(noinst_LTLIBRARIES)
 
 @USE_LIBTOOL_TRUE@libreent_la_OBJECTS =  closer.lo reent.lo impure.lo \
-@USE_LIBTOOL_TRUE@fstatr.lo getreent.lo linkr.lo lseekr.lo openr.lo \
-@USE_LIBTOOL_TRUE@readr.lo signalr.lo signgam.lo sbrkr.lo statr.lo \
-@USE_LIBTOOL_TRUE@timer.lo unlinkr.lo writer.lo
+@USE_LIBTOOL_TRUE@fcntlr.lo fstatr.lo getreent.lo linkr.lo lseekr.lo \
+@USE_LIBTOOL_TRUE@openr.lo readr.lo signalr.lo signgam.lo sbrkr.lo \
+@USE_LIBTOOL_TRUE@statr.lo timer.lo unlinkr.lo writer.lo
 CFLAGS = @CFLAGS@
 COMPILE = $(CC) $(DEFS) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS)
 LTCOMPILE = $(LIBTOOL) --mode=compile $(CC) $(DEFS) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS)
Index: libc/reent/fcntlr.c
===================================================================
RCS file: libc/reent/fcntlr.c
diff -N libc/reent/fcntlr.c
--- libc/reent/fcntlr.c	1 Jan 1970 00:00:00 -0000
+++ libc/reent/fcntlr.c	17 Sep 2002 19:50:27 -0000
@@ -0,0 +1,65 @@
+/* Reentrant versions of fcntl system call.  This implementation just
+   calls the fcntl system call.  */
+
+#include <reent.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <_syslist.h>
+
+/* Some targets provides their own versions of these functions.  Those
+   targets should define REENTRANT_SYSCALLS_PROVIDED in TARGET_CFLAGS.  */
+
+#ifdef _REENT_ONLY
+#ifndef REENTRANT_SYSCALLS_PROVIDED
+#define REENTRANT_SYSCALLS_PROVIDED
+#endif
+#endif
+
+#ifndef REENTRANT_SYSCALLS_PROVIDED
+
+/* We use the errno variable used by the system dependent layer.  */
+#undef errno
+extern int errno;
+
+/*
+FUNCTION
+	<<_fcntl_r>>---Reentrant version of fcntl
+	
+INDEX
+	_fcntl_r
+
+ANSI_SYNOPSIS
+	#include <reent.h>
+	int _fcntl_r(struct _reent *<[ptr]>,
+		     int <[fd]>, int <[cmd]>, <[arg]>);
+
+TRAD_SYNOPSIS
+	#include <reent.h>
+	int _fcntl_r(<[ptr]>, <[fd]>, <[cmd]>, <[arg]>)
+	struct _reent *<[ptr]>;
+	int <[fd]>;
+	int <[cmd]>;
+	int <[arg]>;
+
+DESCRIPTION
+	This is a reentrant version of <<fcntl>>.  It
+	takes a pointer to the global data block, which holds
+	<<errno>>.
+*/
+
+int
+_fcntl_r (ptr, fd, cmd, arg)
+     struct _reent *ptr;
+     int fd;
+     int cmd;
+     int arg;
+{
+  int ret;
+
+  errno = 0;
+  if ((ret = _fcntl (fd, cmd, arg)) == -1 && errno != 0)
+    ptr->_errno = errno;
+  return ret;
+}
+
+#endif /* ! defined (REENTRANT_SYSCALLS_PROVIDED) */
Index: libc/search/hash.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/search/hash.c,v
retrieving revision 1.3
diff -u -r1.3 hash.c
--- libc/search/hash.c	2 Jul 2002 18:18:58 -0000	1.3
+++ libc/search/hash.c	17 Sep 2002 19:50:27 -0000
@@ -143,7 +143,9 @@
 		     fstat(hashp->fp, &statbuf) == 0 && statbuf.st_size == 0)
 			new_table = 1;
 
+#ifdef HAVE_FCNTL
 		(void)fcntl(hashp->fp, F_SETFD, 1);
+#endif
 	}
 	if (new_table) {
 		if (!(hashp = init_hash(hashp, file, (HASHINFO *)info)))
Index: libc/search/hash_page.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/search/hash_page.c,v
retrieving revision 1.3
diff -u -r1.3 hash_page.c
--- libc/search/hash_page.c	2 Jul 2002 18:18:58 -0000	1.3
+++ libc/search/hash_page.c	17 Sep 2002 19:50:27 -0000
@@ -869,7 +869,9 @@
 	(void)sigprocmask(SIG_BLOCK, &set, &oset);
 	if ((hashp->fp = mkstemp(namestr)) != -1) {
 		(void)unlink(namestr);
+#ifdef HAVE_FCNTL
 		(void)fcntl(hashp->fp, F_SETFD, 1);
+#endif
 	}
 	(void)sigprocmask(SIG_SETMASK, &oset, (sigset_t *)NULL);
 	return (hashp->fp != -1 ? 0 : -1);
Index: libc/stdio/fdopen.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fdopen.c,v
retrieving revision 1.5
diff -u -r1.5 fdopen.c
--- libc/stdio/fdopen.c	21 Feb 2001 23:11:06 -0000	1.5
+++ libc/stdio/fdopen.c	17 Sep 2002 19:50:27 -0000
@@ -64,7 +64,7 @@
 
   /* make sure the mode the user wants is a subset of the actual mode */
 #ifdef HAVE_FCNTL
-  if ((fdflags = _fcntl (fd, F_GETFL, 0)) < 0)
+  if ((fdflags = _fcntl_r (ptr, fd, F_GETFL, 0)) < 0)
     return 0;
   fdmode = fdflags & O_ACCMODE;
   if (fdmode != O_RDWR && (fdmode != (oflags & O_ACCMODE)))
Index: libc/syscalls/sysfcntl.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/syscalls/sysfcntl.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 sysfcntl.c
--- libc/syscalls/sysfcntl.c	17 Feb 2000 19:39:51 -0000	1.1.1.1
+++ libc/syscalls/sysfcntl.c	17 Sep 2002 19:50:28 -0000
@@ -2,6 +2,7 @@
 /* only called from stdio/fdopen.c, so arg can be int. */
 
 #include <reent.h>
+#include <errno.h>
 
 int
 fcntl (fd, flag, arg)
@@ -9,9 +10,14 @@
      int flag;
      int arg;
 {
-#ifdef REENTRANT_SYSCALLS_PROVIDED
+#ifdef HAVE_FCNTL
+# ifdef REENTRANT_SYSCALLS_PROVIDED
   return _fcntl_r (_REENT, fd, flag, arg);
-#else
+# else
   return _fcntl (fd, flag, arg);
-#endif
+# endif
+#else /* !HAVE_FCNTL */
+  errno = ENOSYS;
+  return -1;
+#endif /& !HAVE_FCNTL */
 }