Re: copy_file_preserving variant that doesn't exit on error?

[Reuben Thomas]

Ping! I've not heard anything about this; my current patch (as used in
GNU Zile) still seems to work.

On 2 August 2011 18:31, Reuben Thomas <address@hidden> wrote:
> Here's a revised patch to provide error returning copy-file as well as
> error-raising copy-file.
>
> Of particular interest:
>
> 1. What would you like to call the new function?
>
> 2. I haven't yet added a test; from looking at the tests for copy-file
> it seems it should suffice to add a call to remove followed by an
> asserted call to copy_file_preserving_error (or whatever it's called)
> after the call to copy_file_preserving, i.e. just repeat the copy with
> the other version of the function.
>
> diff --git a/lib/copy-file.c b/lib/copy-file.c
> index f9cd9c0..4ae8427 100644
> --- a/lib/copy-file.c
> +++ b/lib/copy-file.c
> @@ -53,47 +53,79 @@
>
>  enum { IO_SIZE = 32 * 1024 };
>
> -void
> -copy_file_preserving (const char *src_filename, const char *dest_filename)
> +enum {
> +  ERR_OPEN_READ = 1,
> +  ERR_OPEN_BACKUP_WRITE,
> +  ERR_READ,
> +  ERR_WRITE,
> +  ERR_AFTER_READ,
> +  ERR_ACL
> +};
> +
> +
> +int
> +_copy_file_preserving (const char *src_filename, const char *dest_filename)
>  {
>   int src_fd;
>   struct stat statbuf;
>   int mode;
>   int dest_fd;
> +  int err = 0;
>   char *buf = xmalloc (IO_SIZE);
>
>   src_fd = open (src_filename, O_RDONLY | O_BINARY);
> -  if (src_fd < 0 || fstat (src_fd, &statbuf) < 0)
> -    error (EXIT_FAILURE, errno, _("error while opening \"%s\" for reading"),
> -           src_filename);
> +  if (src_fd < 0)
> +    {
> +      err = ERR_OPEN_READ;
> +      goto error;
> +    }
> +  if (fstat (src_fd, &statbuf) < 0)
> +    {
> +      err = ERR_OPEN_READ;
> +      goto error_src;
> +    }
>
>   mode = statbuf.st_mode & 07777;
>
>   dest_fd = open (dest_filename, O_WRONLY | O_CREAT | O_TRUNC |
> O_BINARY, 0600);
>   if (dest_fd < 0)
> -    error (EXIT_FAILURE, errno, _("cannot open backup file \"%s\" for
> writing"),
> -           dest_filename);
> +    {
> +      err = ERR_OPEN_BACKUP_WRITE;
> +      goto error_src;
> +    }
>
>   /* Copy the file contents.  */
>   for (;;)
>     {
>       size_t n_read = safe_read (src_fd, buf, IO_SIZE);
>       if (n_read == SAFE_READ_ERROR)
> -        error (EXIT_FAILURE, errno, _("error reading \"%s\""), src_filename);
> +        {
> +          err = ERR_READ;
> +          goto error_src_dest;
> +        }
>       if (n_read == 0)
>         break;
>
>       if (full_write (dest_fd, buf, n_read) < n_read)
> -        error (EXIT_FAILURE, errno, _("error writing \"%s\""), 
> dest_filename);
> +        {
> +          err = ERR_WRITE;
> +          goto error_src_dest;
> +        }
>     }
>
>   free (buf);
>
>  #if !USE_ACL
>   if (close (dest_fd) < 0)
> -    error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
> +    {
> +      err = ERR_WRITE;
> +      goto error_src;
> +    }
>   if (close (src_fd) < 0)
> -    error (EXIT_FAILURE, errno, _("error after reading \"%s\""), 
> src_filename);
> +    {
> +      err = ERR_AFTER_READ;
> +      goto error;
> +    }
>  #endif
>
>   /* Preserve the access and modification times.  */
> @@ -123,15 +155,73 @@ copy_file_preserving (const char *src_filename,
> const char *dest_filename)
>   /* Preserve the access permissions.  */
>  #if USE_ACL
>   if (copy_acl (src_filename, src_fd, dest_filename, dest_fd, mode))
> -    exit (EXIT_FAILURE);
> +    {
> +      err = ERR_ACL;
> +      goto error_src_dest;
> +    }
>  #else
>   chmod (dest_filename, mode);
>  #endif
>
>  #if USE_ACL
>   if (close (dest_fd) < 0)
> -    error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
> +    {
> +      err = ERR_WRITE;
> +      goto error_src;
> +    }
>   if (close (src_fd) < 0)
> -    error (EXIT_FAILURE, errno, _("error after reading \"%s\""), 
> src_filename);
> +    {
> +      err = ERR_AFTER_READ;
> +      goto error;
> +    }
>  #endif
> +
> +  return 0;
> +
> + error_src_dest:
> +  close (dest_fd);
> + error_src:
> +  close (src_fd);
> + error:
> +  return -err;
> +}
> +
> +void
> +copy_file_preserving (const char *src_filename, const char *dest_filename)
> +{
> +  switch (_copy_file_preserving (src_filename, dest_filename))
> +    {
> +    case 0:
> +      return;
> +
> +    case ERR_OPEN_READ:
> +      error (EXIT_FAILURE, errno, _("error while opening \"%s\" for 
> reading"),
> +             src_filename);
> +
> +    case ERR_OPEN_BACKUP_WRITE:
> +      error (EXIT_FAILURE, errno, _("cannot open backup file \"%s\"
> for writing"),
> +             dest_filename);
> +
> +    case ERR_READ:
> +      error (EXIT_FAILURE, errno, _("error reading \"%s\""), src_filename);
> +
> +    case ERR_WRITE:
> +      error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
> +
> +    case ERR_AFTER_READ:
> +      error (EXIT_FAILURE, errno, _("error after reading \"%s\""),
> src_filename);
> +
> +    case ERR_ACL:
> +      exit (EXIT_FAILURE);
> +
> +    default:
> +      exit (EXIT_FAILURE);
> +    }
> +}
> +
> +
> +int
> +copy_file_preserving_error (const char *src_filename, const char
> *dest_filename)
> +{
> +  return _copy_file_preserving (src_filename, dest_filename) < 0 ? - 1 : 0;
>  }
> diff --git a/lib/copy-file.h b/lib/copy-file.h
> index cb8b1f7..0f0931c 100644
> --- a/lib/copy-file.h
> +++ b/lib/copy-file.h
> @@ -28,6 +28,9 @@ extern "C" {
>    Exit upon failure.  */
>  extern void copy_file_preserving (const char *src_filename, const
> char *dest_filename);
>
> +/* Error-returning version of copy_file_preserving. */
> +extern int copy_file_preserving_error (const char *src_filename,
> const char *dest_filename);
> +
>
>  #ifdef __cplusplus
>  }



-- 
http://rrt.sc3d.org