Hi,
The patch will remove FIXME item from cp, install, ln and mv. I do
acknowledge that getenv ("SIMPLE_BACKUP_SUFFIX") does still get to be
called unnecessarily if numbered backups are asked, which is not
perfect, but better than calling it always.
--
Sami Kerola
http://www.iki.fi/kerolasa/
From c73a80e2b8b29564122df4c85a17d4bf5cdabf8b Mon Sep 17 00:00:00 2001
From: Sami Kerola <[email protected]>
Date: Sat, 13 Nov 2010 18:17:05 +0100
Subject: [PATCH] cp/install/ln/mv: call getenv SIMPLE_BACKUP_SUFFIX only when
backups are defined
call getenv SIMPLE_BACKUP_SUFFIX only when backups are defined
* src/cp.c: Likewise.
* src/install.c: Likewise.
* src/ln.c: Likewise.
* src/mv.c: Likewise.
Signed-off-by: Sami Kerola <[email protected]>
---
src/cp.c | 15 +++++++--------
src/install.c | 15 +++++++--------
src/ln.c | 15 +++++++--------
src/mv.c | 15 +++++++--------
4 files changed, 28 insertions(+), 32 deletions(-)
diff --git a/src/cp.c b/src/cp.c
index 5b14f3a..1a6552c 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -905,7 +905,7 @@ main (int argc, char **argv)
int c;
bool ok;
bool make_backups = false;
- char *backup_suffix_string;
+ char *backup_suffix_string = NULL;
char *version_control_string = NULL;
struct cp_options x;
bool copy_contents = false;
@@ -923,10 +923,6 @@ main (int argc, char **argv)
selinux_enabled = (0 < is_selinux_enabled ());
cp_option_init (&x);
- /* FIXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
- we'll actually use backup_suffix_string. */
- backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
-
while ((c = getopt_long (argc, argv, "abdfHilLnprst:uvxPRS:T",
long_opts, NULL))
!= -1)
@@ -1116,14 +1112,17 @@ main (int argc, char **argv)
usage (EXIT_FAILURE);
}
- 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);
+ if (make_backups && backup_suffix_string == NULL)
+ backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
+
+ if (backup_suffix_string)
+ simple_backup_suffix = xstrdup (backup_suffix_string);
+
if (x.dereference == DEREF_UNDEFINED)
{
if (x.recursive)
diff --git a/src/install.c b/src/install.c
index 467e500..9c48fc9 100644
--- a/src/install.c
+++ b/src/install.c
@@ -427,7 +427,7 @@ main (int argc, char **argv)
int exit_status = EXIT_SUCCESS;
const char *specified_mode = NULL;
bool make_backups = false;
- char *backup_suffix_string;
+ char *backup_suffix_string = NULL;
char *version_control_string = NULL;
bool mkdir_and_install = false;
struct cp_options x;
@@ -456,10 +456,6 @@ main (int argc, char **argv)
dir_arg = false;
umask (0);
- /* FIXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
- we'll actually use backup_suffix_string. */
- backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
-
while ((optc = getopt_long (argc, argv, "bcCsDdg:m:o:pt:TvS:Z:",
long_options,
NULL)) != -1)
{
@@ -574,14 +570,17 @@ main (int argc, char **argv)
_("cannot force target context to %s and preserve it"),
quote (scontext));
- 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);
+ if (make_backups && backup_suffix_string == NULL)
+ backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
+
+ if (backup_suffix_string)
+ simple_backup_suffix = xstrdup (backup_suffix_string);
+
if (scontext && setfscreatecon (scontext) < 0)
error (EXIT_FAILURE, errno,
_("failed to set default file creation context to %s"),
diff --git a/src/ln.c b/src/ln.c
index 672964c..01c53c0 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -407,7 +407,7 @@ main (int argc, char **argv)
int c;
bool ok;
bool make_backups = false;
- char *backup_suffix_string;
+ char *backup_suffix_string = NULL;
char *version_control_string = NULL;
char const *target_directory = NULL;
bool no_target_directory = false;
@@ -422,10 +422,6 @@ main (int argc, char **argv)
atexit (close_stdin);
- /* FIXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
- we'll actually use backup_suffix_string. */
- backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
-
symbolic_link = remove_existing_files = interactive = verbose
= hard_dir_link = false;
@@ -532,13 +528,16 @@ main (int argc, char **argv)
quote (file[n_files - 1]));
}
- if (backup_suffix_string)
- simple_backup_suffix = xstrdup (backup_suffix_string);
-
backup_type = (make_backups
? xget_version (_("backup type"), version_control_string)
: no_backups);
+ if (make_backups && backup_suffix_string == NULL)
+ backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
+
+ if (backup_suffix_string)
+ simple_backup_suffix = xstrdup (backup_suffix_string);
+
if (target_directory)
{
int i;
diff --git a/src/mv.c b/src/mv.c
index 91aadfa..f50e382 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -342,7 +342,7 @@ main (int argc, char **argv)
int c;
bool ok;
bool make_backups = false;
- char *backup_suffix_string;
+ char *backup_suffix_string = NULL;
char *version_control_string = NULL;
struct cp_options x;
char *target_directory = NULL;
@@ -363,10 +363,6 @@ main (int argc, char **argv)
/* Try to disable the ability to unlink a directory. */
priv_set_remove_linkdir ();
- /* FIXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
- we'll actually use backup_suffix_string. */
- backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
-
while ((c = getopt_long (argc, argv, "bfint:uvS:T", long_options, NULL))
!= -1)
{
@@ -465,14 +461,17 @@ main (int argc, char **argv)
usage (EXIT_FAILURE);
}
- 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);
+ if (make_backups && backup_suffix_string == NULL)
+ backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
+
+ if (backup_suffix_string)
+ simple_backup_suffix = xstrdup (backup_suffix_string);
+
hash_init ();
if (target_directory)
--
1.7.3.2
