Here's a proposed series of tweaks and alignments to the 'acl' and 'copy-file'
modules, to make Reuben's proposed change to 'copy-file' easier.
The first change is not backward compatible, but the only users of copy_acl()
are coreutils and 'copy-file' in gnulib, and I've verified that these callers
don't care whether the return value is -1 or -2.
For the third change, I considered exporting just one function copy_acl
with a null_stderr argument, like in the module 'wait-process', rather than
two function qcopy_acl and copy_acl. But I find the approach with two
functions more maintainable: It would make it easier to move the code
that does not emit error message to an LGPLed module.
Objections?
2012-01-10 Bruno Haible <br...@clisp.org>
copy-file: Use 'quote' module consistently.
* lib/copy-file.c (copy_file_preserving): Use quote().
copy-file: Refactor.
* lib/copy-file.c: Include quote.h.
(copy_file_preserving): Call qcopy_acl instead of copy_acl. Emit error
message here.
* modules/copy-file (Depends-on): Add quote.
acl: Export qcopy_acl.
* lib/acl.h (qcopy_acl): New declaration.
* lib/copy-acl.c (qcopy_acl): Make non-static.
acl: Tweak.
* lib/set-mode-acl.c (set_acl): Use same variable name as in copy_acl.
acl: Align return values of copy_acl and qcopy_acl.
* lib/copy-acl.c (copy_acl): Return the same value as qcopy_acl,
maybe < -1.
>From 321956903be57d1e10cdb63774ddf57635a9693b Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Wed, 11 Jan 2012 01:51:25 +0100
Subject: [PATCH 1/5] acl: Align return values of copy_acl and qcopy_acl.
* lib/copy-acl.c (copy_acl): Return the same value as qcopy_acl,
maybe < -1.
---
ChangeLog | 6 ++++++
lib/copy-acl.c | 10 ++++++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f302d88..d7c6fd4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2012-01-10 Bruno Haible <br...@clisp.org>
+ acl: Align return values of copy_acl and qcopy_acl.
+ * lib/copy-acl.c (copy_acl): Return the same value as qcopy_acl,
+ maybe < -1.
+
+2012-01-10 Bruno Haible <br...@clisp.org>
+
regex: Avoid link error on MSVC 9.
* modules/regex (Depends-on): Add wctype.
diff --git a/lib/copy-acl.c b/lib/copy-acl.c
index 195da98..a318396 100644
--- a/lib/copy-acl.c
+++ b/lib/copy-acl.c
@@ -621,7 +621,8 @@ qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
If access control lists are not available, fchmod the target file to
MODE. Also sets the non-permission bits of the destination file
(S_ISUID, S_ISGID, S_ISVTX) to those from MODE if any are set.
- Return 0 if successful, otherwise output a diagnostic and return -1. */
+ Return 0 if successful, otherwise output a diagnostic and return a
+ negative error code. */
int
copy_acl (const char *src_name, int source_desc, const char *dst_name,
@@ -632,13 +633,14 @@ copy_acl (const char *src_name, int source_desc, const char *dst_name,
{
case -2:
error (0, errno, "%s", quote (src_name));
- return -1;
+ break;
case -1:
error (0, errno, _("preserving permissions for %s"), quote (dst_name));
- return -1;
+ break;
default:
- return 0;
+ break;
}
+ return ret;
}
--
1.6.3.2
>From eb43217bc90b138509132d5185de43434e33120d Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Wed, 11 Jan 2012 01:52:23 +0100
Subject: [PATCH 2/5] acl: Tweak.
* lib/set-mode-acl.c (set_acl): Use same variable name as in copy_acl.
---
ChangeLog | 3 +++
lib/set-mode-acl.c | 6 +++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index d7c6fd4..ca333ec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2012-01-10 Bruno Haible <br...@clisp.org>
+ acl: Tweak.
+ * lib/set-mode-acl.c (set_acl): Use same variable name as in copy_acl.
+
acl: Align return values of copy_acl and qcopy_acl.
* lib/copy-acl.c (copy_acl): Return the same value as qcopy_acl,
maybe < -1.
diff --git a/lib/set-mode-acl.c b/lib/set-mode-acl.c
index b9d202e..a81b321 100644
--- a/lib/set-mode-acl.c
+++ b/lib/set-mode-acl.c
@@ -677,8 +677,8 @@ qset_acl (char const *name, int desc, mode_t mode)
int
set_acl (char const *name, int desc, mode_t mode)
{
- int r = qset_acl (name, desc, mode);
- if (r != 0)
+ int ret = qset_acl (name, desc, mode);
+ if (ret != 0)
error (0, errno, _("setting permissions for %s"), quote (name));
- return r;
+ return ret;
}
--
1.6.3.2
>From f067b0cbf5c86b5613a14fff55b2500dba5f0771 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Wed, 11 Jan 2012 01:54:25 +0100
Subject: [PATCH 3/5] acl: Export qcopy_acl.
* lib/acl.h (qcopy_acl): New declaration.
* lib/copy-acl.c (qcopy_acl): Make non-static.
---
ChangeLog | 4 ++++
lib/acl.h | 5 +++--
lib/copy-acl.c | 2 +-
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ca333ec..61d4103 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2012-01-10 Bruno Haible <br...@clisp.org>
+ acl: Export qcopy_acl.
+ * lib/acl.h (qcopy_acl): New declaration.
+ * lib/copy-acl.c (qcopy_acl): Make non-static.
+
acl: Tweak.
* lib/set-mode-acl.c (set_acl): Use same variable name as in copy_acl.
diff --git a/lib/acl.h b/lib/acl.h
index 762d3c1..dc36b0d 100644
--- a/lib/acl.h
+++ b/lib/acl.h
@@ -21,7 +21,8 @@
#include <sys/stat.h>
int file_has_acl (char const *, struct stat const *);
-int copy_acl (char const *, int, char const *, int, mode_t);
-int set_acl (char const *, int, mode_t);
int qset_acl (char const *, int, mode_t);
+int set_acl (char const *, int, mode_t);
+int qcopy_acl (char const *, int, char const *, int, mode_t);
+int copy_acl (char const *, int, char const *, int, mode_t);
int chmod_or_fchmod (char const *, int, mode_t);
diff --git a/lib/copy-acl.c b/lib/copy-acl.c
index a318396..9b9f033 100644
--- a/lib/copy-acl.c
+++ b/lib/copy-acl.c
@@ -38,7 +38,7 @@
Return -2 and set errno for an error relating to the source file.
Return -1 and set errno for an error relating to the destination file. */
-static int
+int
qcopy_acl (const char *src_name, int source_desc, const char *dst_name,
int dest_desc, mode_t mode)
{
--
1.6.3.2
>From 6962ac880e5c06b9c6b9708ffd156b85ac93af95 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Wed, 11 Jan 2012 02:01:45 +0100
Subject: [PATCH 4/5] copy-file: Refactor.
* lib/copy-file.c: Include quote.h.
(copy_file_preserving): Call qcopy_acl instead of copy_acl. Emit error
message here.
* modules/copy-file (Depends-on): Add quote.
---
ChangeLog | 6 ++++++
lib/copy-file.c | 11 +++++++++--
modules/copy-file | 1 +
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 61d4103..4494496 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2012-01-10 Bruno Haible <br...@clisp.org>
+ copy-file: Refactor.
+ * lib/copy-file.c: Include quote.h.
+ (copy_file_preserving): Call qcopy_acl instead of copy_acl. Emit error
+ message here.
+ * modules/copy-file (Depends-on): Add quote.
+
acl: Export qcopy_acl.
* lib/acl.h (qcopy_acl): New declaration.
* lib/copy-acl.c (qcopy_acl): Make non-static.
diff --git a/lib/copy-file.c b/lib/copy-file.c
index 8e79091..5bd6fa8 100644
--- a/lib/copy-file.c
+++ b/lib/copy-file.c
@@ -41,6 +41,7 @@
#include "full-write.h"
#include "acl.h"
#include "binary-io.h"
+#include "quote.h"
#include "gettext.h"
#include "xalloc.h"
@@ -122,8 +123,14 @@ 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);
+ switch (qcopy_acl (src_filename, src_fd, dest_filename, dest_fd, mode))
+ {
+ case -2:
+ error (EXIT_FAILURE, errno, "%s", quote (src_filename));
+ case -1:
+ error (EXIT_FAILURE, errno, _("preserving permissions for %s"),
+ quote (dest_filename));
+ }
#else
chmod (dest_filename, mode);
#endif
diff --git a/modules/copy-file b/modules/copy-file
index 6a35db2..1c50d55 100644
--- a/modules/copy-file
+++ b/modules/copy-file
@@ -14,6 +14,7 @@ fstat
full-write
gettext-h
open
+quote
safe-read
stdlib
unistd
--
1.6.3.2
>From 0c66d103e861b504172649e27168031f92e1fd33 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Wed, 11 Jan 2012 02:11:34 +0100
Subject: [PATCH 5/5] copy-file: Use 'quote' module consistently.
* lib/copy-file.c (copy_file_preserving): Use quote().
---
ChangeLog | 3 +++
lib/copy-file.c | 24 ++++++++++++++----------
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4494496..a578854 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2012-01-10 Bruno Haible <br...@clisp.org>
+ copy-file: Use 'quote' module consistently.
+ * lib/copy-file.c (copy_file_preserving): Use quote().
+
copy-file: Refactor.
* lib/copy-file.c: Include quote.h.
(copy_file_preserving): Call qcopy_acl instead of copy_acl. Emit error
diff --git a/lib/copy-file.c b/lib/copy-file.c
index 5bd6fa8..c4d8600 100644
--- a/lib/copy-file.c
+++ b/lib/copy-file.c
@@ -65,36 +65,39 @@ copy_file_preserving (const char *src_filename, const char *dest_filename)
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);
+ error (EXIT_FAILURE, errno, _("error while opening %s for reading"),
+ quote (src_filename));
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);
+ error (EXIT_FAILURE, errno, _("cannot open backup file %s for writing"),
+ quote (dest_filename));
/* 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);
+ error (EXIT_FAILURE, errno, _("error reading %s"),
+ quote (src_filename));
if (n_read == 0)
break;
if (full_write (dest_fd, buf, n_read) < n_read)
- error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
+ error (EXIT_FAILURE, errno, _("error writing %s"),
+ quote (est_filename));
}
free (buf);
#if !USE_ACL
if (close (dest_fd) < 0)
- error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
+ error (EXIT_FAILURE, errno, _("error writing %s"), quote (dest_filename));
if (close (src_fd) < 0)
- error (EXIT_FAILURE, errno, _("error after reading \"%s\""), src_filename);
+ error (EXIT_FAILURE, errno, _("error after reading %s"),
+ quote (src_filename));
#endif
/* Preserve the access and modification times. */
@@ -137,8 +140,9 @@ copy_file_preserving (const char *src_filename, const char *dest_filename)
#if USE_ACL
if (close (dest_fd) < 0)
- error (EXIT_FAILURE, errno, _("error writing \"%s\""), dest_filename);
+ error (EXIT_FAILURE, errno, _("error writing %s"), quote (dest_filename));
if (close (src_fd) < 0)
- error (EXIT_FAILURE, errno, _("error after reading \"%s\""), src_filename);
+ error (EXIT_FAILURE, errno, _("error after reading %s"),
+ quote (src_filename));
#endif
}
--
1.6.3.2
