On Fri, Mar 27, 2009 at 15:20, Sami Kerola <[email protected]> wrote:

> Well I changed rename to copy that is part of coreutils and use the
> call in move_mode. I did not send new patch because I want to be sure
> my employer has signed papers stating that they have no right to my
> contributions to coreutils. Perhaps I should wait copyright papers to
> get ready before making questions. It's a bit difficult to ask
> questions && get correct answers when you don't see what I am talking
> about.

Hello again,

My legal papers are ready to be sent to Boston. I'll be in touch with
GNU copyright personel tomorrow. Since it should be now safe to send
patches here is next one. Quite honestly I am not 100% happy about it.
The patch does quite much everything that you'd expect, but there are
two issues.

When backup is specified permissions and ownership of mkstemp file are
used. It would make more sense to use destination attributes, but the
copy.c does not support that. I tried write a patch which would do
that, but it was tricker than I expected. And if you think this is
right thing to do I'll invest more of my time to write such patch.

The message "The backup suffix is `~', unless..." should go to
lib/version-etc.c or somewhere similar. If in-place is implemented to
several command this sort of tactic should be nice to translators.

So what do you think, is the in-place implementation better or worse
when it uses copy.c?

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/
From 04b61ac97f21498f597f131485c58f629b69865d Mon Sep 17 00:00:00 2001
From: Sami Kerola <[email protected]>
Date: Mon, 13 Apr 2009 17:57:04 +0200
Subject: [PATCH] in-place option that uses copy.c
Cc: [email protected]

---
 src/Makefile.am |    3 +
 src/unexpand.c  |  283 ++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 244 insertions(+), 42 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 9aaf739..d869aaa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -102,6 +102,7 @@ runcon_LDADD = $(LDADD) $(LIB_SELINUX)
 pathchk_LDADD = $(LDADD) $(LIB_EACCESS)
 rm_LDADD = $(LDADD) $(LIB_EACCESS)
 test_LDADD = $(LDADD) $(LIB_EACCESS)
+unexpand_LDADD = $(LDADD) $(LIB_EACCESS) $(LIB_SELINUX)
 # This is for the '[' program.  Automake transliterates '[' to '_'.
 __LDADD = $(LDADD) $(LIB_EACCESS)
 
@@ -155,6 +156,7 @@ vdir_LDADD += $(LIB_ACL)
 cp_LDADD += $(LIB_ACL) $(LIB_XATTR)
 mv_LDADD += $(LIB_ACL) $(LIB_XATTR)
 ginstall_LDADD += $(LIB_ACL) $(LIB_XATTR)
+unexpand += $(LIB_ACL) $(LIB_XATTR)
 
 stat_LDADD = $(LDADD) $(LIB_SELINUX)
 
@@ -265,6 +267,7 @@ mkdir_SOURCES = mkdir.c prog-fprintf.c
 rmdir_SOURCES = rmdir.c prog-fprintf.c
 
 uname_SOURCES = uname.c uname-uname.c
+unexpand_SOURCES = unexpand.c $(copy_sources)
 arch_SOURCES = uname.c uname-arch.c
 
 md5sum_SOURCES = md5sum.c
diff --git a/src/unexpand.c b/src/unexpand.c
index 93f1ce8..0da7d68 100644
--- a/src/unexpand.c
+++ b/src/unexpand.c
@@ -38,10 +38,16 @@
 #include <stdio.h>
 #include <getopt.h>
 #include <sys/types.h>
+#include <selinux/selinux.h>
+#include <signal.h>
+
 #include "system.h"
 #include "error.h"
 #include "quote.h"
 #include "xstrndup.h"
+#include "backupfile.h"
+#include "copy.h"
+#include "cp-hash.h"
 
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "unexpand"
@@ -85,8 +91,23 @@ static bool have_read_stdin;
 /* The desired exit status.  */
 static int exit_status;
 
-/* Write back file if true */
-static bool in_place;
+/* Use SA_NOCLDSTOP as a proxy for whether the sigaction machinery is
+   present.  */
+#ifndef SA_NOCLDSTOP
+# define SA_NOCLDSTOP 0
+/* No sigprocmask.  Always 'return' zero. */
+# define sigprocmask(How, Set, Oset) (0)
+# define sigset_t int
+# if ! HAVE_SIGINTERRUPT
+#  define siginterrupt(sig, flag) /* empty */
+# endif
+#endif
+
+/* The set of signals that are caught.  */
+static sigset_t caught_signals;
+
+/* Write back file(s) if true */
+static bool in_place = false;
 
 /* For long options that have no equivalent short option, use a
    non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
@@ -101,15 +122,18 @@ struct in_out_files
   FILE *infile;
   FILE *outfile;
   char *temp;
-  struct stat st;
 };
+struct in_out_files *iofs;
 
 static struct option const longopts[] =
 {
   {"tabs", required_argument, NULL, 't'},
   {"all", no_argument, NULL, 'a'},
   {"first-only", no_argument, NULL, CONVERT_FIRST_ONLY_OPTION},
+  {"output", required_argument, NULL, 'o'},
   {"in-place", no_argument, NULL, IN_PLACE_OPTION},
+  {"backup", optional_argument, NULL, 'b'},
+  {"suffix", required_argument, NULL, 'S'},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -142,16 +166,69 @@ Mandatory arguments to long options are mandatory for 
short options too.\n\
   -t, --tabs=LIST  use comma separated LIST of tab positions (enables -a)\n\
 "), stdout);
       fputs (_("\
-      --in-place   edit files in place\n\
+      --backup[=CONTROL]  make a backup of each existing destination file\n\
+  -b                      like --backup but does not accept an argument\n\
+  -S, --suffix=SUFFIX     override the usual backup suffix\n\
+      --in-place          edit files in place\n\
 "), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
+      fputs (_("\
+\n\
+The backup suffix is `~', unless set with --suffix or SIMPLE_BACKUP_SUFFIX.\n\
+The version control method may be selected via the --backup option or 
through\n\
+the VERSION_CONTROL environment variable.  Here are the values:\n\
+\n\
+"), stdout);
+      fputs (_("\
+  none, off       never make backups (even if --backup is given)\n\
+  numbered, t     make numbered backups\n\
+  existing, nil   numbered if numbered backups exist, simple otherwise\n\
+  simple, never   always make simple backups\n\
+"), stdout);
       emit_bug_reporting_address ();
     }
   exit (status);
 }
 
-/* Add tab stop TABVAL to the end of `tab_list'.  */
+static void
+cp_option_init (struct cp_options *x)
+{
+  bool selinux_enabled = (0 < is_selinux_enabled ());
+
+  cp_options_default (x);
+  x->copy_as_regular = false;
+  x->dereference = DEREF_ALWAYS;
+  x->unlink_dest_before_opening = false;
+  x->unlink_dest_after_failed_open = false;
+  x->hard_link = false;
+  x->interactive = I_UNSPECIFIED;
+  x->move_mode = true;
+  x->one_file_system = false;
+  x->preserve_ownership = false;
+  x->preserve_links = true;
+  x->preserve_mode = false;
+  x->preserve_timestamps = false;
+  if (selinux_enabled)
+    x->preserve_security_context = true;
+  x->reduce_diagnostics = false;
+  x->require_preserve = false;
+  x->require_preserve_context = false;
+  x->preserve_xattr = false;
+  x->require_preserve_xattr = false;
+  x->recursive = false;
+  x->sparse_mode = SPARSE_AUTO;
+  x->symbolic_link = false;
+  x->set_mode = false;
+  x->mode = 0;
+  x->stdin_tty = isatty (STDIN_FILENO);
+
+  x->open_dangling_dest_symlink = false;
+  x->update = false;
+  x->verbose = false;
+  x->dest_info = NULL;
+  x->src_info = NULL;
+}
 
 static void
 add_tab_stop (uintmax_t tabval)
@@ -251,12 +328,12 @@ validate_tab_stops (uintmax_t const *tabs, size_t entries)
    Return NULL if there are no more input files.  */
 
 static struct in_out_files *
-next_file (struct in_out_files *iofs)
+next_file (struct in_out_files *iofs, const struct cp_options *x)
 {
   static char *prev_file;
   char *file;
-  char pattern[] = "unexpandXXXXXX";
-  char *tmpdir = NULL;
+  bool copy_into_self;
+  bool rename_succeeded;
 
   if (iofs->infile)
     {
@@ -274,20 +351,18 @@ next_file (struct in_out_files *iofs)
              error (0, errno, "%s", prev_file);
              exit_status = EXIT_FAILURE;
             }
-          if (in_place && (fclose (iofs->outfile) != 0))
+          if (in_place)
             {
-              error (0, errno, "%s", iofs->temp);
-              exit_status = EXIT_FAILURE;
-            }
-          if (rename(iofs->temp, prev_file))
-            {
-              error (0, errno, "moving a temporary file in place failed");
-              exit_status = EXIT_FAILURE;
-            }
-          if (chmod(prev_file, iofs->st.st_mode))
-            {
-              error (0, errno, "resetting file mode failed");
-              exit_status = EXIT_FAILURE;
+              if(fclose(iofs->outfile))
+                {
+                  error (0, errno, "$s", iofs->temp);
+                  exit_status = EXIT_FAILURE;
+                }
+              /* Return value of copy is insignificant in this case, the
+                 source is always a file. */
+              copy (iofs->temp, prev_file, false, x, &copy_into_self,
+                &rename_succeeded);
+              strncpy(iofs->temp, "XXXXXX", strlen(iofs->temp) - 6);
             }
         }
     }
@@ -305,24 +380,11 @@ next_file (struct in_out_files *iofs)
          return iofs;
        }
       iofs->infile = fopen (file, "r");
-      fstat(fileno (iofs->infile), &(iofs->st));
       if (iofs->infile)
        {
          prev_file = file;
          if (in_place)
            {
-              if (! ((tmpdir=getenv("TMPDIR"))))
-                {
-                  tmpdir=getenv("TMP");
-                }
-              if (!tmpdir)
-                {
-                  tmpdir = "/tmp";
-                }
-              iofs->temp = xmalloc(strlen(tmpdir) + strlen(pattern) + 2);
-              strcpy(iofs->temp, tmpdir);
-              strcat(iofs->temp, "/");
-              strcat(iofs->temp, pattern);
              if (mkstemp(iofs->temp) == 0 ||
                (iofs->outfile = fopen(iofs->temp, "w")) == NULL)
                {
@@ -345,14 +407,32 @@ next_file (struct in_out_files *iofs)
    Read each file in `file_list', in order.  */
 
 static void
-unexpand (void)
+unexpand (const struct cp_options *x)
 {
-  struct in_out_files *iofs;
+  char *tmpdir = NULL;
+  char pattern[] = "XXXXXX";
   iofs = xmalloc (sizeof(struct in_out_files));
   iofs->infile = NULL;
 
+  if (in_place)
+    {
+      if (! ((tmpdir=getenv("TMPDIR"))))
+        {
+          tmpdir=getenv("TMP");
+        }
+      if (!tmpdir)
+        {
+          tmpdir = "/tmp";
+        }
+       iofs->temp = xmalloc(strlen(tmpdir) + strlen(program_name) +
+         strlen(pattern) + 2);
+       strcpy(iofs->temp, tmpdir);
+       strcat(iofs->temp, "/");
+       strcat(iofs->temp, program_name);
+       strcat(iofs->temp, pattern);
+    }
   /* Input stream.  */
-  iofs = next_file (iofs);
+  iofs = next_file (iofs, x);
 
   /* The array of pending blanks.  In non-POSIX locales, blanks can
      include characters other than spaces, so the blanks must be
@@ -403,7 +483,7 @@ unexpand (void)
 
       do
        {
-         while ((c = getc (iofs->infile)) < 0 && (iofs = next_file (iofs)))
+         while ((c = getc (iofs->infile)) < 0 && (iofs = next_file (iofs, x)))
            continue;
 
          if (convert)
@@ -511,14 +591,50 @@ unexpand (void)
        }
       while (c != '\n');
     }
-    free (iofs->temp);
+}
+
+void
+cleanup(void)
+{
+  struct in_out_files volatile *local = (struct in_out_files *) &iofs;
+  sigset_t sigs;
+
+  /* Try protect entering to cleanup several times.  */
+  sigprocmask (SIG_BLOCK, &caught_signals, &sigs);
+   
+  if (in_place && local->temp)
+    {
+      /* Usually non-existing is deleted, not very smart.  */
+      unlink (local->temp);
+      free (local->temp);
+    }
+  if (iofs)
     free (iofs);
+
+  fclose(stdout);
+}
+
+void
+sighandler (int sig)
+{
+  if (! SA_NOCLDSTOP)
+    signal (sig, SIG_IGN);
+
+  cleanup ();
+  
+  signal (sig, SIG_DFL);
+  raise(sig);
 }
 
 int
 main (int argc, char **argv)
 {
   bool have_tabval = false;
+  bool make_backups = false;
+  char *outfile = NULL;
+  char *backup_suffix_string = NULL;
+  char *version_control_string = NULL;
+  struct cp_options x;
   in_place = false;
   uintmax_t tabval IF_LINT (= 0);
   int c;
@@ -533,7 +649,61 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
 
-  atexit (close_stdout);
+  {
+    size_t i;
+    static int const sig[] =
+      {
+       /* The usual suspects.  */
+       SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
+#ifdef SIGPOLL
+       SIGPOLL,
+#endif
+#ifdef SIGPROF
+       SIGPROF,
+#endif
+#ifdef SIGVTALRM
+       SIGVTALRM,
+#endif
+#ifdef SIGXCPU
+       SIGXCPU,
+#endif
+#ifdef SIGXFSZ
+       SIGXFSZ,
+#endif
+      };
+    enum { nsigs = sizeof sig / sizeof sig[0] };
+
+#if SA_NOCLDSTOP
+    struct sigaction act;
+
+    sigemptyset (&caught_signals);
+    for (i = 0; i < nsigs; i++)
+      {
+       sigaction (sig[i], NULL, &act);
+       if (act.sa_handler != SIG_IGN)
+         sigaddset (&caught_signals, sig[i]);
+      }
+
+    act.sa_handler = sighandler;
+    act.sa_mask = caught_signals;
+    act.sa_flags = 0;
+
+    for (i = 0; i < nsigs; i++)
+      if (sigismember (&caught_signals, sig[i]))
+       sigaction (sig[i], &act, NULL);
+#else
+    for (i = 0; i < nsigs; i++)
+      if (signal (sig[i], SIG_IGN) != SIG_IGN)
+       {
+         signal (sig[i], sighandler);
+         siginterrupt (sig[i], 1);
+       }
+#endif
+  }
+
+  atexit (cleanup);
+
+  cp_option_init(&x);
 
   have_read_stdin = false;
   exit_status = EXIT_SUCCESS;
@@ -541,7 +711,7 @@ main (int argc, char **argv)
   tab_list = NULL;
   first_free_tab = 0;
 
-  while ((c = getopt_long (argc, argv, ",0123456789at:", longopts, NULL))
+  while ((c = getopt_long (argc, argv, ",0123456789at:o:", longopts, NULL))
         != -1)
     {
       switch (c)
@@ -551,10 +721,24 @@ main (int argc, char **argv)
        case 'a':
          convert_entire_line = true;
          break;
+        case 'b':
+          make_backups = true;
+          if (optarg)
+            version_control_string = optarg;
+          break;
+        case 'o':
+          if (outfile && !STREQ (outfile, optarg))
+            error (EXIT_FAILURE, 0, _("multiple output files specified"));
+          outfile = optarg;
+          break;
        case 't':
          convert_entire_line = true;
          parse_tab_stops (optarg);
          break;
+        case 'S':
+          make_backups = true;
+          backup_suffix_string = optarg;
+          break;
        case CONVERT_FIRST_ONLY_OPTION:
          convert_first_only = true;
          break;
@@ -580,6 +764,21 @@ main (int argc, char **argv)
        }
     }
 
+  /* Backups are meaningful only when output is going to a file. */
+  if (in_place)
+    {
+      backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
+      if (backup_suffix_string)
+        simple_backup_suffix = xstrdup (backup_suffix_string);
+    
+      x.backup_type = (make_backups
+                       ? xget_version (_("backup type"),
+                                       version_control_string)
+                       : no_backups);
+    }
+
+  hash_init ();
+
   if (convert_first_only)
     convert_entire_line = false;
 
@@ -597,7 +796,7 @@ main (int argc, char **argv)
 
   file_list = (optind < argc ? &argv[optind] : stdin_argv);
 
-  unexpand ();
+  unexpand (&x);
 
   if (have_read_stdin && fclose (stdin) != 0)
     error (EXIT_FAILURE, errno, "-");
-- 
1.6.2

_______________________________________________
Bug-coreutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Reply via email to