cpio suggestion + patch

[Taylor Gautier]

I have a suggestion for cpio.  The suggestion is to make it copy files into
a temporary name and then rename the file as the last operation.  Since UNIX
filesystems are supposed to gaurantee atomicity at the rename level, this
should be a safer way of copying/creating files.  

Using the old method if something happened to the cpio process, or the pipe
it is reading from, during the middle of a write, cpio could create
corrupted files.

I have an included a patch that makes this improvement.   I wasn't sure
about a number of things:

1) Whether this enhancement should be an option.  On the one hand, this
method is far safer, on the other it could leave temp files around.

2) I didn't feel like looking into all the exit points of the copyin
routine, so instead of freeing the memory I allocate for a temp name, I just
realloc the pointer every time through the loop.  This will "leak" the last
filename that is created, but this seems ok in light cpio's use (i.e. not a
daemon)

3) I wasn't sure how to ensure non-UNIX compatibility.  I create a temp
filename based on the original filename by looking for a '/', aka UNIX style
path seperator.  I am not sure this will work in a non-UNIX environment.

4) Since the rename should be atomic, I removed the unlink of an existing
file.  Not sure if this is correct behavior for all cases.

So here is the patch, maybe it can inspire someone to fix up the issues I
have highlighted, since I am not really sure how to solve them.

Taylor Gautier

--- copyin.c.orig       Fri Apr 20 09:21:32 2001
+++ copyin.c    Fri Apr 20 09:26:35 2001
@@ -385,6 +385,7 @@
   int existing_dir;            /* True if file is a dir & already exists.
*/
   int i;                       /* Loop index variable.  */
   char *link_name = NULL;      /* Name of hard and symbolic links.  */
+  char *unique = NULL;          /* Unique name of file */
 #ifdef HPUX_CDF
   int cdf_flag;                 /* True if file is a CDF.  */
   int cdf_char;                 /* Index of `+' char indicating a CDF.  */
@@ -641,7 +642,7 @@
                }
              else if (S_ISDIR (file_stat.st_mode)
                        ? rmdir (file_hdr.c_name)
-                       : unlink (file_hdr.c_name))
+                       : 0)
                {
                  error (0, errno, "cannot remove current %s",
                         file_hdr.c_name);
@@ -729,14 +730,37 @@
              /* If not linked, copy the contents of the file.  */
              if (link_name == NULL)
                {
-                 out_file_des = open (file_hdr.c_name,
-                                      O_CREAT | O_WRONLY | O_BINARY, 0600);
+                  /* Compute a temporary name for the file based
+                   * on the original name.
+                   *
+                   * For filename /x/y/z/foo.txt -> /x/y/z/.foo.txt.XXXXXX
+                   * The trailing XXXXXX is for mkstemp.
+                   *
+                   * This "leaks" the last filename, but cpio is called
+                   * as a batch process, not long running, so should be
+                   * ok.
+                   */
+                  char * pos;
+                 unique = realloc(unique, file_hdr.c_namesize + 8);
+                  memcpy(unique, file_hdr.c_name, file_hdr.c_namesize);
+                  pos = rindex(unique, '/');
+                  if (pos == NULL)
+                    {
+                      pos = unique;
+                    }
+                  else
+                    {
+                      pos++;
+                    }
+                  memmove(pos+1, pos, unique + file_hdr.c_namesize - pos -
1);
+                  *pos = '.';
+                  strcat(unique, ".XXXXXX");
+
+                  out_file_des = mkstemp(unique);
                  if (out_file_des < 0 && create_dir_flag)
                    {
                      create_all_directories (file_hdr.c_name);
-                     out_file_des = open (file_hdr.c_name,
-                                          O_CREAT | O_WRONLY | O_BINARY,
-                                          0600);
+                      out_file_des = mkstemp(unique);
                    }
                  if (out_file_des < 0)
                    {
@@ -765,8 +789,6 @@
                    }
                  copy_files_tape_to_disk (in_file_des, out_file_des,
file_hdr.c_filesize);
                  disk_empty_output_buffer (out_file_des);
-                   error (0, errno, "%s", file_hdr.c_name);

                  if (archive_format == arf_crcascii)
                    {
@@ -774,22 +796,36 @@
                        error (0, 0, "%s: checksum error (0x%x, should be
0x%x)",
                               file_hdr.c_name, crc, file_hdr.c_chksum);
                    }
+
                  /* File is now copied; set attributes.  */
+
                  if (!no_chown_flag)
-                   if ((chown (file_hdr.c_name,
-                               set_owner_flag ? set_owner : file_hdr.c_uid,
+                   if ((fchown (out_file_des,
+                                set_owner_flag ? set_owner :
file_hdr.c_uid,
                           set_group_flag ? set_group : file_hdr.c_gid) < 0)
                        && errno != EPERM)
                      error (0, errno, "%s", file_hdr.c_name);
                  /* chown may have turned off some permissions we wanted.
*/
-                 if (chmod (file_hdr.c_name, (int) file_hdr.c_mode) < 0)
+                 if (fchmod (out_file_des, (int) file_hdr.c_mode) < 0)
                    error (0, errno, "%s", file_hdr.c_name);
                  if (retain_time_flag)
                    {
                      times.actime = times.modtime = file_hdr.c_mtime;
-                     if (utime (file_hdr.c_name, &times) < 0)
+                     if (utime (unique, &times) < 0)
                        error (0, errno, "%s", file_hdr.c_name);
                    }
+
+                  /* Attributes are set, close descriptor */
+                 if (close (out_file_des) < 0)
+                   error (0, errno, "%s", file_hdr.c_name);
+
+                  /* File is now done; rename.  */
+                  if (rename(unique, file_hdr.c_name) < 0)
+                    {
+                      error(0, 0, "cannot rename %s to %s",
+                            unique, file_hdr.c_name);
+                    }
+
                  tape_skip_padding (in_file_des, file_hdr.c_filesize);
                  if (file_hdr.c_nlink > 1 && (archive_format ==
arf_newascii
                      || archive_format == arf_crcascii) )