Did fix that - changing function call to GNU style and "suffix_from_env" to
local variable. The corresponding patch is attached.
On Mon, Apr 4, 2016 at 10:36 PM, Paul Eggert <[email protected]> wrote:
> On 04/04/2016 09:30 AM, Rishabh Dave wrote:
>
>> + char *suffix_from_env;
>> size_t ssize;
>> bool simple = true;
>> +
>> + /* If simple_backup_suffix is '~', check environment if we have any
>> there. */
>> + if (strcmp(simple_backup_suffix, "~") == 0)
>> + if ((suffix_from_env = getenv("SIMPLE_BACKUP_SUFFIX")) != NULL)
>> + simple_backup_suffix = xstrdup (suffix_from_env);
>> +
>>
>
> The usual GNU style is "f (x)" rather than "f(x)" for function calls.
> Also, avoid side effects in an "if"; just have the then-part with a local
> variable "suffix_from_env" and put the subsidary if after that.
>
diff -ur Original/coreutils-8.25/lib/backupfile.c Debugging/coreutils-8.25/lib/backupfile.c
--- Original/coreutils-8.25/lib/backupfile.c 2016-01-01 19:15:55.000000000 +0530
+++ Debugging/coreutils-8.25/lib/backupfile.c 2016-04-05 20:11:12.096568278 +0530
@@ -268,6 +268,15 @@
size_t ssize;
bool simple = true;
+
+ /* If simple_backup_suffix is '~', check environment if we have any there. */
+ if (strcmp (simple_backup_suffix, "~") == 0)
+ {
+ char *suffix_from_env;
+ if ((suffix_from_env = getenv ("SIMPLE_BACKUP_SUFFIX")) != NULL)
+ simple_backup_suffix = xstrdup (suffix_from_env);
+ }
+
/* Allow room for simple or ".~N~" backups. The guess must be at
least sizeof ".~1~", but otherwise will be adjusted as needed. */
size_t simple_backup_suffix_size = strlen (simple_backup_suffix) + 1;
diff -ur Original/coreutils-8.25/src/cp.c Debugging/coreutils-8.25/src/cp.c
--- Original/coreutils-8.25/src/cp.c 2016-01-01 19:18:50.000000000 +0530
+++ Debugging/coreutils-8.25/src/cp.c 2016-04-02 18:29:20.284884000 +0530
@@ -920,7 +920,7 @@
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;
@@ -939,10 +939,6 @@
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)
diff -ur Original/coreutils-8.25/src/install.c Debugging/coreutils-8.25/src/install.c
--- Original/coreutils-8.25/src/install.c 2016-01-03 18:29:44.000000000 +0530
+++ Debugging/coreutils-8.25/src/install.c 2016-04-02 18:31:36.856884000 +0530
@@ -784,7 +784,7 @@
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;
@@ -813,10 +813,6 @@
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)
{
diff -ur Original/coreutils-8.25/src/ln.c Debugging/coreutils-8.25/src/ln.c
--- Original/coreutils-8.25/src/ln.c 2016-01-01 19:18:50.000000000 +0530
+++ Debugging/coreutils-8.25/src/ln.c 2016-04-02 18:31:11.764884000 +0530
@@ -464,7 +464,7 @@
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;
@@ -479,10 +479,6 @@
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;
diff -ur Original/coreutils-8.25/src/mv.c Debugging/coreutils-8.25/src/mv.c
--- Original/coreutils-8.25/src/mv.c 2016-01-12 17:11:44.000000000 +0530
+++ Debugging/coreutils-8.25/src/mv.c 2016-04-02 18:30:24.832884000 +0530
@@ -346,7 +346,7 @@
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;
@@ -368,10 +368,6 @@
/* 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)
{
