bug#23153: [PATCH]: For FIXME in cp.c

[Pádraig Brady]

On 29/03/16 16:28, Rishabh Dave wrote:
> Hello,
>
> I have wrote the attached patch for following FIXME in file src/cp.c -
>
> /* FIXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
>       we'll actually use backup_suffix_string.  */
>    backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");
>
> Since we use backup_suffix_string to duplicate it into
> simple_backup_suffix, I brought the getenv() call there, that too, only if
> required (as simple_backup_suffix already stores tilde already).
>
>
> I did 'diff -ur'  directly against original cp.c (named cp-original.c,
> then) to create the patch. I tested patch using -b, --backup and --suffix
> option of c. Version I have used is latest one on savannah.gnu.org -
> coreuitls-8.25.
>
> There was cppi (didn't know what it does) at the bottom of coreutil's
> download page. After reading it's README, I concluded that it is not to be
> considered while debugging/fixing coreutils. Hopefully, I was correct in
> doing so.
>
> If incorrect, please do correct me in any case. :)
>
> There was one little doubt, maybe bug, after doing '--backups=numbered' it
> becomes impossible to have suffixed backups (using --suffix or -b) until we
> do '--backup=simple' explicitly. Is this supposed to be? I tried with both
> altered and unaltered version of cp.
> (And should I have or should create/d a new separate thread? I wasn't sure.)


Thanks for the patch.
This doesn't address the FIXME though as we still getenv() in the normal case.
To fix that would be more invasive and may need changes to gnulib's backupfile 
module.
Also any changes here should also be done for mv, ln and install.


thanks,
Pádraig