Attached are the two patches I intend to push for this upon the next gnulib update.
Marking this bug as done. thanks, Pádraig
>From 2535447f85c4ca5b7caea0bf144e125267e0fa95 Mon Sep 17 00:00:00 2001 From: Rishabh Dave <[email protected]> Date: Wed, 2 Nov 2016 23:43:47 +0000 Subject: [PATCH 1/2] maint: simplify handling of backup --suffix in various tools * src/cp.c (main): Avoid the getenv("SIMPLE_BACKUP_SUFFIX") call, which is now done if needed in the gnulib backupfile module. Also avoid the redundant strdup, as we don't modify this suffix. * src/install.c (main): Likewise. * src/ln.c (main): Likewise. * src/mv.c (main): Likewise. Fixes http://bugs.gnu.org/23153 --- src/cp.c | 10 +--------- src/install.c | 10 +--------- src/ln.c | 10 +--------- src/mv.c | 10 +--------- 4 files changed, 4 insertions(+), 36 deletions(-) diff --git a/src/cp.c b/src/cp.c index b25c9ce..79b93bd 100644 --- a/src/cp.c +++ b/src/cp.c @@ -922,7 +922,6 @@ main (int argc, char **argv) int c; bool ok; bool make_backups = false; - char *backup_suffix_string; char *version_control_string = NULL; struct cp_options x; bool copy_contents = false; @@ -941,10 +940,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:TZ", long_opts, NULL)) != -1) @@ -1123,7 +1118,7 @@ main (int argc, char **argv) case 'S': make_backups = true; - backup_suffix_string = optarg; + simple_backup_suffix = optarg; break; case_GETOPT_HELP_CHAR; @@ -1154,9 +1149,6 @@ 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) diff --git a/src/install.c b/src/install.c index 9182e50..b4b282a 100644 --- a/src/install.c +++ b/src/install.c @@ -807,7 +807,6 @@ main (int argc, char **argv) int exit_status = EXIT_SUCCESS; const char *specified_mode = NULL; bool make_backups = false; - char *backup_suffix_string; char *version_control_string = NULL; bool mkdir_and_install = false; struct cp_options x; @@ -836,10 +835,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) { @@ -889,7 +884,7 @@ main (int argc, char **argv) break; case 'S': make_backups = true; - backup_suffix_string = optarg; + simple_backup_suffix = optarg; break; case 't': if (target_directory) @@ -961,9 +956,6 @@ main (int argc, char **argv) quoteaf (target_directory)); } - if (backup_suffix_string) - simple_backup_suffix = xstrdup (backup_suffix_string); - x.backup_type = (make_backups ? xget_version (_("backup type"), version_control_string) diff --git a/src/ln.c b/src/ln.c index 618b03d..0b8eb21 100644 --- a/src/ln.c +++ b/src/ln.c @@ -465,7 +465,6 @@ main (int argc, char **argv) int c; bool ok; bool make_backups = false; - char *backup_suffix_string; char *version_control_string = NULL; char const *target_directory = NULL; bool no_target_directory = false; @@ -480,10 +479,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; @@ -547,7 +542,7 @@ main (int argc, char **argv) break; case 'S': make_backups = true; - backup_suffix_string = optarg; + simple_backup_suffix = optarg; break; case_GETOPT_HELP_CHAR; case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); @@ -594,9 +589,6 @@ main (int argc, char **argv) quoteaf (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); diff --git a/src/mv.c b/src/mv.c index 25fa8a4..35b2e92 100644 --- a/src/mv.c +++ b/src/mv.c @@ -347,7 +347,6 @@ main (int argc, char **argv) int c; bool ok; bool make_backups = false; - char *backup_suffix_string; char *version_control_string = NULL; struct cp_options x; char *target_directory = NULL; @@ -369,10 +368,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:TZ", long_options, NULL)) != -1) { @@ -421,7 +416,7 @@ main (int argc, char **argv) break; case 'S': make_backups = true; - backup_suffix_string = optarg; + simple_backup_suffix = optarg; break; case 'Z': /* As a performance enhancement, don't even bother trying @@ -481,9 +476,6 @@ 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) -- 2.5.5 >From 8e348bce903bf71518fc9ecbb7b0e20ea321b751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Wed, 2 Nov 2016 23:56:53 +0000 Subject: [PATCH 2/2] maint: refactor printing of backup suffix --help * src/system.h (emit_backup_suffix_note): A new function to output the backup suffix info. The strings are unchanged, so translations are not impacted. * src/cp.c (usage): Use the new function. * src/ln.c (usage): Likewise. * src/mv.c (usage): Likewise. * src/install.c (usage): Likewise. --- src/cp.c | 14 +------------- src/install.c | 14 +------------- src/ln.c | 14 +------------- src/mv.c | 14 +------------- src/system.h | 18 ++++++++++++++++++ 5 files changed, 22 insertions(+), 52 deletions(-) diff --git a/src/cp.c b/src/cp.c index 79b93bd..97a868a 100644 --- a/src/cp.c +++ b/src/cp.c @@ -250,19 +250,7 @@ When --reflink[=always] is specified, perform a lightweight copy, where the\n\ data blocks are copied only when modified. If this is not possible the copy\n\ fails, or if --reflink=auto is specified, fall back to a standard copy.\n\ "), 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_backup_suffix_note (); fputs (_("\ \n\ As a special case, cp makes a backup of SOURCE when the force and backup\n\ diff --git a/src/install.c b/src/install.c index b4b282a..4fa4bb3 100644 --- a/src/install.c +++ b/src/install.c @@ -681,19 +681,7 @@ In the 4th form, create all components of the given DIRECTORY(ies).\n\ 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_backup_suffix_note (); emit_ancillary_info (PROGRAM_NAME); } exit (status); diff --git a/src/ln.c b/src/ln.c index 0b8eb21..2a56dc9 100644 --- a/src/ln.c +++ b/src/ln.c @@ -436,19 +436,7 @@ interpreted in relation to its parent directory.\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_backup_suffix_note (); printf (_("\ \n\ Using -s ignores -L and -P. Otherwise, the last option specified controls\n\ diff --git a/src/mv.c b/src/mv.c index 35b2e92..6a3d0d2 100644 --- a/src/mv.c +++ b/src/mv.c @@ -323,19 +323,7 @@ If you specify more than one of -i, -f, -n, only the final one takes effect.\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_backup_suffix_note (); emit_ancillary_info (PROGRAM_NAME); } exit (status); diff --git a/src/system.h b/src/system.h index 1b7a0fb..e82dce4 100644 --- a/src/system.h +++ b/src/system.h @@ -609,6 +609,24 @@ Otherwise, units default to 1024 bytes (or 512 if POSIXLY_CORRECT is set).\n\ } static inline void +emit_backup_suffix_note (void) +{ + 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); +} + +static inline void emit_ancillary_info (char const *program) { struct infomap { char const *program; char const *node; } const infomap[] = { -- 2.5.5
