Re: ping [PATCH]: Allow _rename_r to be re-classified as a system call, [PATCH]: Remove "is closed check" in fclose.c and fwalk.c

[Jeff Johnston <jjohnstn at redhat dot com>]

Antony KING wrote:
> Hi,
>
> Its been a while :-) since I posted these patches so I thought I might
> try again.
>
> The original submissions can be found on the newlib mailing list at the
> following URLs:
>
> http://www.sourceware.org/ml/newlib/2006/msg00872.html

For the rename problem, I propose an alternate patch.  This treats it 
like fcntl.  The existing flags HAVE_RENAME and 
REENTRANT_SYSCALLS_PROVIDED flags are used.  If you say HAVE_RENAME and 
REENTRANT_SYSCALLS_PROVIDED, then you are assumed to provide _rename_r 
like the other syscalls.  This should not cause any problems for 
existing platforms.

> http://www.sourceware.org/ml/newlib/2006/msg00871.html

Please explain the scenario further.  I don't understand why locking 
and, more importantly, subsequently unlocking the file causes a problem 
for you.

> Even though the patches are against a pre- 1.15.0 version of newlib they
> are still valid against the current wave front.
>
> Cheers,
>
> Antony.

Index: reent/Makefile.am
===================================================================
RCS file: /cvs/src/src/newlib/libc/reent/Makefile.am,v
retrieving revision 1.11
diff -u -p -r1.11 Makefile.am
--- reent/Makefile.am	21 Feb 2007 19:22:33 -0000	1.11
+++ reent/Makefile.am	7 Aug 2007 22:20:43 -0000
@@ -41,6 +41,7 @@ GENERAL_SOURCES = \
 	lseekr.c \
 	openr.c \
 	readr.c \
+	renamer.c \
 	signalr.c \
 	signgam.c \
 	sbrkr.c \
@@ -75,6 +76,7 @@ CHEWOUT_FILES = \
 	lseekr.def \
 	openr.def \
 	readr.def \
+	renamer.def \
 	signalr.def \
 	sbrkr.def \
 	statr.def \
Index: reent/Makefile.in
===================================================================
RCS file: /cvs/src/src/newlib/libc/reent/Makefile.in,v
retrieving revision 1.22
diff -u -p -r1.22 Makefile.in
--- reent/Makefile.in	24 May 2007 17:33:36 -0000	1.22
+++ reent/Makefile.in	7 Aug 2007 22:20:43 -0000
@@ -60,10 +60,11 @@ am__objects_1 = lib_a-closer.$(OBJEXT) l
 	lib_a-fstatr.$(OBJEXT) lib_a-getreent.$(OBJEXT) \
 	lib_a-gettimeofdayr.$(OBJEXT) lib_a-linkr.$(OBJEXT) \
 	lib_a-lseekr.$(OBJEXT) lib_a-openr.$(OBJEXT) \
-	lib_a-readr.$(OBJEXT) lib_a-signalr.$(OBJEXT) \
-	lib_a-signgam.$(OBJEXT) lib_a-sbrkr.$(OBJEXT) \
-	lib_a-statr.$(OBJEXT) lib_a-timesr.$(OBJEXT) \
-	lib_a-unlinkr.$(OBJEXT) lib_a-writer.$(OBJEXT)
+	lib_a-readr.$(OBJEXT) lib_a-renamer.$(OBJEXT) \
+	lib_a-signalr.$(OBJEXT) lib_a-signgam.$(OBJEXT) \
+	lib_a-sbrkr.$(OBJEXT) lib_a-statr.$(OBJEXT) \
+	lib_a-timesr.$(OBJEXT) lib_a-unlinkr.$(OBJEXT) \
+	lib_a-writer.$(OBJEXT)
 @HAVE_STDIO64_DIR_TRUE@am__objects_2 = lib_a-fstat64r.$(OBJEXT) \
 @HAVE_STDIO64_DIR_TRUE@	lib_a-lseek64r.$(OBJEXT) \
 @HAVE_STDIO64_DIR_TRUE@	lib_a-open64r.$(OBJEXT)
@@ -81,8 +82,8 @@ LTLIBRARIES = $(noinst_LTLIBRARIES)
 libreent_la_LIBADD =
 am__objects_6 = closer.lo reent.lo impure.lo fcntlr.lo fstatr.lo \
 	getreent.lo gettimeofdayr.lo linkr.lo lseekr.lo openr.lo \
-	readr.lo signalr.lo signgam.lo sbrkr.lo statr.lo timesr.lo \
-	unlinkr.lo writer.lo
+	readr.lo renamer.lo signalr.lo signgam.lo sbrkr.lo statr.lo \
+	timesr.lo unlinkr.lo writer.lo
 @HAVE_STDIO64_DIR_TRUE@am__objects_7 = fstat64r.lo lseek64r.lo \
 @HAVE_STDIO64_DIR_TRUE@	open64r.lo
 am__objects_8 = $(am__objects_7)
@@ -298,6 +299,7 @@ GENERAL_SOURCES = \
 	lseekr.c \
 	openr.c \
 	readr.c \
+	renamer.c \
 	signalr.c \
 	signgam.c \
 	sbrkr.c \
@@ -325,6 +327,7 @@ CHEWOUT_FILES = \
 	lseekr.def \
 	openr.def \
 	readr.def \
+	renamer.def \
 	signalr.def \
 	sbrkr.def \
 	statr.def \
@@ -470,6 +473,12 @@ lib_a-readr.o: readr.c
 lib_a-readr.obj: readr.c
 	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-readr.obj `if test -f 'readr.c'; then $(CYGPATH_W) 'readr.c'; else $(CYGPATH_W) '$(srcdir)/readr.c'; fi`
 
+lib_a-renamer.o: renamer.c
+	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-renamer.o `test -f 'renamer.c' || echo '$(srcdir)/'`renamer.c
+
+lib_a-renamer.obj: renamer.c
+	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-renamer.obj `if test -f 'renamer.c'; then $(CYGPATH_W) 'renamer.c'; else $(CYGPATH_W) '$(srcdir)/renamer.c'; fi`
+
 lib_a-signalr.o: signalr.c
 	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-signalr.o `test -f 'signalr.c' || echo '$(srcdir)/'`signalr.c
 
Index: reent/renamer.c
===================================================================
RCS file: reent/renamer.c
diff -N reent/renamer.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ reent/renamer.c	7 Aug 2007 22:20:43 -0000
@@ -0,0 +1,74 @@
+/* Reentrant version of rename system call.  */
+
+#include <reent.h>
+#include <rename.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
+	<<_rename_r>>---Reentrant version of rename
+	
+INDEX
+	_rename_r
+
+ANSI_SYNOPSIS
+	#include <reent.h>
+	int _rename_r(struct _reent *<[ptr]>,
+		     int <[fd]>, int <[cmd]>, <[arg]>);
+
+TRAD_SYNOPSIS
+	#include <reent.h>
+	int _rename_r(<[ptr]>, <[fd]>, <[cmd]>, <[arg]>)
+	struct _reent *<[ptr]>;
+	int <[fd]>;
+	int <[cmd]>;
+	int <[arg]>;
+
+DESCRIPTION
+	This is a reentrant version of <<rename>>.  It
+	takes a pointer to the global data block, which holds
+	<<errno>>.
+*/
+
+int
+_DEFUN (_rename_r, (ptr, old, new),
+     struct _reent *ptr _AND
+     _CONST char *old _AND
+     _CONST char *new)
+{
+  int ret = 0;
+
+#ifdef HAVE_RENAME
+  errno = 0;
+  if ((ret = _rename (old, new)) == -1 && errno != 0)
+    ptr->_errno = errno;
+#else
+  if (_link_r (ptr, old, new) == -1)
+    return -1;
+
+  if (_unlink_r (ptr, old) == -1)
+    {
+      /* ??? Should we unlink new? (rhetorical question) */
+      return -1;
+    }
+#endif
+  return ret;
+}
+
+#endif /* ! defined (REENTRANT_SYSCALLS_PROVIDED) */
Index: stdio/rename.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/rename.c,v
retrieving revision 1.2
diff -u -p -r1.2 rename.c
--- stdio/rename.c	23 Apr 2004 20:01:55 -0000	1.2
+++ stdio/rename.c	7 Aug 2007 22:20:43 -0000
@@ -21,27 +21,17 @@ FUNCTION
 
 INDEX
 	rename
-INDEX
-	_rename_r
 
 ANSI_SYNOPSIS
 	#include <stdio.h>
 	int rename(const char *<[old]>, const char *<[new]>);
 
-	int _rename_r(struct _reent *<[reent]>, 
-                      const char *<[old]>, const char *<[new]>);
-
 TRAD_SYNOPSIS
 	#include <stdio.h>
 	int rename(<[old]>, <[new]>)
 	char *<[old]>;
 	char *<[new]>;
 
-	int _rename_r(<[reent]>, <[old]>, <[new]>)
-	struct _reent *<[reent]>;
-	char *<[old]>;
-	char *<[new]>;
-
 DESCRIPTION
 Use <<rename>> to establish a new name (the string at <[new]>) for a
 file now known by the string at <[old]>.  After a successful
@@ -50,9 +40,6 @@ file now known by the string at <[old]>.
 If <<rename>> fails, the file named <<*<[old]>>> is unaffected.  The
 conditions for failure depend on the host operating system.
 
-The alternate function <<_rename_r>> is a reentrant version.  The
-extra argument <[reent]> is a pointer to a reentrancy structure.
-
 RETURNS
 The result is either <<0>> (when successful) or <<-1>> (when the file
 could not be renamed).
@@ -70,27 +57,6 @@ Supporting OS subroutines required: <<li
 #include <stdio.h>
 #include <sys/unistd.h>
 
-int
-_DEFUN(_rename_r, (ptr, old, new),
-       struct _reent *ptr _AND
-       _CONST char *old   _AND
-       _CONST char *new)
-{
-#ifdef HAVE_RENAME
-  return _rename (old,new);
-#else
-  if (_link_r (ptr, old, new) == -1)
-    return -1;
-
-  if (_unlink_r (ptr, old) == -1)
-    {
-      /* ??? Should we unlink new? (rhetorical question) */
-      return -1;
-    }
-#endif
-  return 0;
-}
-
 #ifndef _REENT_ONLY
 
 int