Обсуждение: Removal of win32-specific rename code

Claudio found that his new delete fix for Win32 also works for rename on
files open by other processes, so I have applied the following patch to
remove the special rename() handling on Win32.  What it used to do was
to rename a file to a *.new, release locks, then do the rename to the
main file.  WIth Claudio's fix, this isn't necessary.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/commands/user.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/user.c,v
retrieving revision 1.138
diff -c -c -r1.138 user.c
*** src/backend/commands/user.c    25 Feb 2004 19:41:22 -0000    1.138
--- src/backend/commands/user.c    16 Mar 2004 04:51:18 -0000
***************
*** 140,149 ****
      bufsize = strlen(filename) + 12;
      tempname = (char *) palloc(bufsize);
      snprintf(tempname, bufsize, "%s.%d", filename, MyProcPid);
- #if defined(WIN32) || defined(__CYGWIN__)
-     filename = repalloc(filename, strlen(filename) + 1 + strlen(".new"));
-     strcat(filename, ".new");
- #endif

      oumask = umask((mode_t) 077);
      fp = AllocateFile(tempname, "w");
--- 140,145 ----
***************
*** 291,300 ****
      bufsize = strlen(filename) + 12;
      tempname = (char *) palloc(bufsize);
      snprintf(tempname, bufsize, "%s.%d", filename, MyProcPid);
- #if defined(WIN32) || defined(__CYGWIN__)
-     filename = repalloc(filename, strlen(filename) + 1 + strlen(".new"));
-     strcat(filename, ".new");
- #endif

      oumask = umask((mode_t) 077);
      fp = AllocateFile(tempname, "w");
--- 287,292 ----
***************
*** 466,483 ****
          user_file_update_needed = false;
          write_user_file(urel);
          heap_close(urel, NoLock);
- #if defined(WIN32) || defined(__CYGWIN__)
-         {
-             /* Rename active file while not holding an exclusive lock */
-             char *filename = user_getfilename(), *filename_new;
-
-             filename_new = palloc(strlen(filename) + 1 + strlen(".new"));
-             sprintf(filename_new, "%s.new", filename);
-             rename(filename_new, filename);
-             pfree(filename);
-             pfree(filename_new);
-         }
- #endif
      }

      if (group_file_update_needed)
--- 458,463 ----
***************
*** 485,502 ****
          group_file_update_needed = false;
          write_group_file(grel);
          heap_close(grel, NoLock);
- #if defined(WIN32) || defined(__CYGWIN__)
-         {
-             /* Rename active file while not holding an exclusive lock */
-             char *filename = group_getfilename(), *filename_new;
-
-             filename_new = palloc(strlen(filename) + 1 + strlen(".new"));
-             sprintf(filename_new, "%s.new", filename);
-             rename(filename_new, filename);
-             pfree(filename);
-             pfree(filename_new);
-         }
- #endif
      }

      /*
--- 465,470 ----
Index: src/backend/utils/cache/relcache.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/cache/relcache.c,v
retrieving revision 1.199
diff -c -c -r1.199 relcache.c
*** src/backend/utils/cache/relcache.c    14 Mar 2004 23:41:27 -0000    1.199
--- src/backend/utils/cache/relcache.c    16 Mar 2004 04:51:25 -0000
***************
*** 3278,3297 ****
           * OK, rename the temp file to its final name, deleting any
           * previously-existing init file.
           */
- #if defined(WIN32) || defined(__CYGWIN__)
          rename(tempfilename, finalfilename);
          LWLockRelease(RelCacheInitLock);
- #else
-         {
-             char        finalfilename_new[MAXPGPATH];
-
-             snprintf(finalfilename_new, sizeof(finalfilename_new), "%s.new", finalfilename);
-             rename(tempfilename, finalfilename_new);
-             LWLockRelease(RelCacheInitLock);
-             /* Rename to active file after lock is released */
-             rename(finalfilename_new, finalfilename);
-         }
- #endif
      }
      else
      {
--- 3278,3285 ----

> main file.  WIth Claudio's fix, this isn't necessary.

Bruce, are you sure this is true? The fix is only for files than are
open()'d, not fopen()'d (as per AllocateFile).

Cheers,
Claudio

---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

Claudio Natoli wrote:
>
> > main file.  WIth Claudio's fix, this isn't necessary.
>
> Bruce, are you sure this is true? The fix is only for files than are
> open()'d, not fopen()'d (as per AllocateFile).

If your fix works, I will change the Win32-specific code to use open()
and fdopen().  The only advantage to the old code was that it would
release locks, then go into a rename loop, which is horid, and we need
to avoid the loop.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

> If your fix works, I will change the Win32-specific code to use open()
> and fdopen().  The only advantage to the old code was that it would
> release locks, then go into a rename loop, which is horid, and we need
> to avoid the loop.

Ah, I see. Should've been obvious you had that in mind actually. Great!

Cheers,
Claudio

---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>