From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 29 17:10:56 2016 Received: (at submit) by debbugs.gnu.org; 29 Mar 2016 21:10:56 +0000 Received: from localhost ([127.0.0.1]:44151 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1al0ud-0002ow-Q3 for submit@debbugs.gnu.org; Tue, 29 Mar 2016 17:10:56 -0400 Received: from eggs.gnu.org ([208.118.235.92]:55090) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1akvZ6-0008I2-2N for submit@debbugs.gnu.org; Tue, 29 Mar 2016 11:28:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akvYw-0003I1-Sw for submit@debbugs.gnu.org; Tue, 29 Mar 2016 11:28:15 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,FREEMAIL_FROM, HTML_MESSAGE,T_DKIM_INVALID autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:36993) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akvYw-0003Hx-Os for submit@debbugs.gnu.org; Tue, 29 Mar 2016 11:28:10 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46027) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akvYs-0001NJ-D6 for bug-coreutils@gnu.org; Tue, 29 Mar 2016 11:28:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akvYo-0003G0-51 for bug-coreutils@gnu.org; Tue, 29 Mar 2016 11:28:06 -0400 Received: from mail-yw0-x234.google.com ([2607:f8b0:4002:c05::234]:33627) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akvYn-0003Fk-VP for bug-coreutils@gnu.org; Tue, 29 Mar 2016 11:28:02 -0400 Received: by mail-yw0-x234.google.com with SMTP id h65so22187894ywe.0 for ; Tue, 29 Mar 2016 08:28:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to; bh=Xf9X+3Y0Sf147roltr6xbwRZHKg4+nB7TMiZ7k58Qi4=; b=Wha/4g0dUipxqsj/sPnzF6QiStl0JoCvnKPavwM2G4iPpoIZyEb3x6CoBAT0bl6y6q ZXL0IlPvD5bNJw+Nj9VY+UTlmHdL/or0skRqqmaJxuSdSWdUvPms4fi3AYKG7fJ4QF8a EkdvvAIVWFDPWrpVmrUKfqngl0K3jkmrSIrBMZ8hDyJ6RaAmlWAyPgHTp9xPGH/iHO7N fi9egPw7uv/F3cbL6DM1GcGqhr6mSDCTJrl6/S/JMtqUDbxni3924o0R4bfB5/l8TVMT wYkxnl8Fnh5pvQSIT1h8rn1KINxX2m6VZuRdX9O35L9i/Z9qr3vElWf6FkdW47mGrV+R rnrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:date:message-id:subject:from:to; bh=Xf9X+3Y0Sf147roltr6xbwRZHKg4+nB7TMiZ7k58Qi4=; b=AoDxvtV/MlAZsjGnXLTAMEh2N3MQq1odawfUf9dN80nNjjofawm9OS8fBvJeh28EC1 Vj6gb3safxiAVtN3jKDnjDR1bNGu5UlsculAnWd7XLw8LF/5t2qf4Qlj0/0TGxQbO40U C4ZnwWdcC5OiG/PpWKx0uh7MyrO5Fa4Q9CGYL0QFZStpobJ3bvcYQL/JdCns003qtj/Q T4so0pbSTvF4QSSO/IsA7QuzNb6EwTgS1mu/OrpMPEkAIetnKK2Jic67w4e+pUX+B4bu U1bq3kYhGDnY51WFJpDxWFHa28dW5wDnqIDdzNj63IdeEgCbmFWc1LLFXFIZ5Y4u/ZJ6 GfDA== X-Gm-Message-State: AD7BkJIbdAAzpUhpm9PWcuRGwpkJiepaKev1taXsGwI4RpxZb9qXtoYkUmEk1JaAutffeUpHjg6zamK1mcB4iQ== MIME-Version: 1.0 X-Received: by 10.202.214.82 with SMTP id n79mr1512239oig.81.1459265281177; Tue, 29 Mar 2016 08:28:01 -0700 (PDT) Received: by 10.60.58.232 with HTTP; Tue, 29 Mar 2016 08:28:01 -0700 (PDT) Date: Tue, 29 Mar 2016 20:58:01 +0530 Message-ID: Subject: [PATCH]: For FIXME in cp.c From: Rishabh Dave To: bug-coreutils@gnu.org Content-Type: multipart/mixed; boundary=001a113b10e8b873fe052f31acf7 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.0 (----) X-Debbugs-Envelope-To: submit X-Mailman-Approved-At: Tue, 29 Mar 2016 17:10:54 -0400 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.0 (----) --001a113b10e8b873fe052f31acf7 Content-Type: multipart/alternative; boundary=001a113b10e8b873f9052f31acf5 --001a113b10e8b873f9052f31acf5 Content-Type: text/plain; charset=UTF-8 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.) --001a113b10e8b873f9052f31acf5 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hello,

I have wrote = the attached patch for following FIXME in file src/cp.c -

/* F= IXME: consider not calling getenv for SIMPLE_BACKUP_SUFFIX unless
=C2=A0= =C2=A0=C2=A0=C2=A0 we'll actually use backup_suffix_string.=C2=A0 */=C2=A0 backup_suffix_string =3D 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 requir= ed (as simple_backup_suffix already stores tilde already).


I did 'diff -ur'=C2=A0 directly against original cp.c (named cp-o= riginal.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.2= 5.

There was cppi (didn't know what it does) at the bottom of co= reutil'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 &#= 39;--backups=3Dnumbered' it becomes impossible to have suffixed backups= (using --suffix or -b) until we do '--backup=3Dsimple' 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.)
--001a113b10e8b873f9052f31acf5-- --001a113b10e8b873fe052f31acf7 Content-Type: text/x-patch; charset=US-ASCII; name="FIXME-calling-getnev.patch" Content-Disposition: attachment; filename="FIXME-calling-getnev.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_imdjksab0 LS0tIGNwLW9yaWdpbmFsLmMJMjAxNi0wMS0wMSAxOToxODo1MC4wMDAwMDAwMDAgKzA1MzAKKysr IGNwLWVkaXRlZC5jCTIwMTYtMDMtMjkgMjA6MTM6NDcuMjc0MzYyMTAwICswNTMwCkBAIC05MjAs NyArOTIwLDcgQEAKICAgaW50IGM7CiAgIGJvb2wgb2s7CiAgIGJvb2wgbWFrZV9iYWNrdXBzID0g ZmFsc2U7Ci0gIGNoYXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5nOworICBjaGFyICpiYWNrdXBfc3Vm Zml4X3N0cmluZyA9IE5VTEw7CiAgIGNoYXIgKnZlcnNpb25fY29udHJvbF9zdHJpbmcgPSBOVUxM OwogICBzdHJ1Y3QgY3Bfb3B0aW9ucyB4OwogICBib29sIGNvcHlfY29udGVudHMgPSBmYWxzZTsK QEAgLTkzOSwxMCArOTM5LDYgQEAKICAgc2VsaW51eF9lbmFibGVkID0gKDAgPCBpc19zZWxpbnV4 X2VuYWJsZWQgKCkpOwogICBjcF9vcHRpb25faW5pdCAoJngpOwogCi0gIC8qIEZJWE1FOiBjb25z aWRlciBub3QgY2FsbGluZyBnZXRlbnYgZm9yIFNJTVBMRV9CQUNLVVBfU1VGRklYIHVubGVzcwot ICAgICB3ZSdsbCBhY3R1YWxseSB1c2UgYmFja3VwX3N1ZmZpeF9zdHJpbmcuICAqLwotICBiYWNr dXBfc3VmZml4X3N0cmluZyA9IGdldGVudiAoIlNJTVBMRV9CQUNLVVBfU1VGRklYIik7Ci0KICAg d2hpbGUgKChjID0gZ2V0b3B0X2xvbmcgKGFyZ2MsIGFyZ3YsICJhYmRmSGlsTG5wcnN0OnV2eFBS UzpUWiIsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICBsb25nX29wdHMsIE5VTEwpKQogICAg ICAgICAgIT0gLTEpCkBAIC0xMTUxLDYgKzExNDcsMTEgQEAKICAgICAgIHVzYWdlIChFWElUX0ZB SUxVUkUpOwogICAgIH0KIAorICAvKiBJZiBiYWNrdXBfc3VmZml4X3N0cmluZyBpcyBzdGlsbCBu dWxsIChpLmUuIG5vIC0tc3VmZml4IG9yIC0tYmFja3VwID0gCisgICAgIG51bWJlcmVkIHwgZXhp c3RpbmcpLCB0cnkgdG8gZ2V0IGl0IGZyb20gZW52aXJvbm1lbnQuICovCisgIGlmICghYmFja3Vw X3N1ZmZpeF9zdHJpbmcpCisgICAgYmFja3VwX3N1ZmZpeF9zdHJpbmcgPSBnZXRlbnYgKCJTSU1Q TEVfQkFDS1VQX1NVRkZJWCIpOworCiAgIGlmIChiYWNrdXBfc3VmZml4X3N0cmluZykKICAgICBz aW1wbGVfYmFja3VwX3N1ZmZpeCA9IHhzdHJkdXAgKGJhY2t1cF9zdWZmaXhfc3RyaW5nKTsKIAo= --001a113b10e8b873fe052f31acf7-- From debbugs-submit-bounces@debbugs.gnu.org Sun Apr 03 05:03:07 2016 Received: (at 23153) by debbugs.gnu.org; 3 Apr 2016 09:03:07 +0000 Received: from localhost ([127.0.0.1]:49427 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1amdw3-0005Ds-1F for submit@debbugs.gnu.org; Sun, 03 Apr 2016 05:03:07 -0400 Received: from mail.magicbluesmoke.com ([82.195.144.49]:45775) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1amdw2-0005Dk-0q for 23153@debbugs.gnu.org; Sun, 03 Apr 2016 05:03:06 -0400 Received: from localhost.localdomain (unknown [109.77.143.219]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.magicbluesmoke.com (Postfix) with ESMTPSA id 1C1EB98A4; Sun, 3 Apr 2016 10:03:04 +0100 (IST) Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c To: Rishabh Dave , 23153@debbugs.gnu.org References: From: =?UTF-8?Q?P=c3=a1draig_Brady?= Message-ID: <5700DC47.7080405@draigBrady.com> Date: Sun, 3 Apr 2016 10:03:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 23153 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) 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 From debbugs-submit-bounces@debbugs.gnu.org Mon Apr 04 12:30:28 2016 Received: (at 23153) by debbugs.gnu.org; 4 Apr 2016 16:30:28 +0000 Received: from localhost ([127.0.0.1]:51303 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1an7OV-0005hB-Ka for submit@debbugs.gnu.org; Mon, 04 Apr 2016 12:30:28 -0400 Received: from mail-ob0-f182.google.com ([209.85.214.182]:33474) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1an7OS-0005gx-RK for 23153@debbugs.gnu.org; Mon, 04 Apr 2016 12:30:25 -0400 Received: by mail-ob0-f182.google.com with SMTP id x3so175484341obt.0 for <23153@debbugs.gnu.org>; Mon, 04 Apr 2016 09:30:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=Jbk/ItkOtkMdux11ryew0LNkQk1AqDcavpo+hQF4PeA=; b=KTSIJjPZoiXnpf93W3R+CaEzj2hTsxcx9Fdg8wECbnuT/QTlkfZwYCOxU5kbj66IY2 2zrurqX0/Cgskv1ytu1iNumk+b8/osbNHA3ch0E6pChPYn+aglm6PhGEWh+WOaFX0NO/ 8EVW+df+7Kp9Max4MU/aFcpCr+kZ0SR3tKTW3VJUJhqCf1PZVgEbwXqiChhq8QDw6x1A vLf4LB7gBAXagli1wWElIYn0MFNdTVvoIiduqFPugTNFBXW68Dt/4mhv0x7bDzrEURQi a7hzmHtC3Cvhbj5XXZS43+bT53AOe99gMRIY5+sT+SfKoPr9fF1DhildMs9sdZitSGJV x5/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=Jbk/ItkOtkMdux11ryew0LNkQk1AqDcavpo+hQF4PeA=; b=h17Qp76EJoLO4Io2xbXkVTJV06rwFtnsxu8x5ckeVs9e8zH0A9UGhB0une+H1++MJ0 rq/mpG6ZaxhsqtV5z2Ps0ywBiYao3X3h468MgV+F+jXu/2/plIXN2hehHGZ7rHOiFHS7 fVDjwPDSD9o7WErFxVLl/uiTWrfyo4wiPEBwGwTlxhZ7C0pkDaG1rL7hQI5VYaL8MtkD DZiBoOlpVpOMYlT553pJMhOAPWu1+sydN9nV/hVe3HgizIW3obC+NsWDf8yNB6qKB4Hw 6Af86HveAi1o/tXd0PGrKHqqHxnAILuLe7jj54DTHG1JTOx2y8Xqi6m8wrrc8Z5fkyzz IPBg== X-Gm-Message-State: AD7BkJJiYWruupQ2Jg6qWXm/4rDpBBcvQ5bMrldnkIGld1/J+d8ncqRyXDY3HAbCmlhGvnO4U/hULqoSIwIVQA== MIME-Version: 1.0 X-Received: by 10.60.101.199 with SMTP id fi7mr11408562oeb.20.1459787419353; Mon, 04 Apr 2016 09:30:19 -0700 (PDT) Received: by 10.60.141.101 with HTTP; Mon, 4 Apr 2016 09:30:19 -0700 (PDT) In-Reply-To: <5700DC47.7080405@draigBrady.com> References: <5700DC47.7080405@draigBrady.com> Date: Mon, 4 Apr 2016 22:00:19 +0530 Message-ID: Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c From: Rishabh Dave To: =?UTF-8?Q?P=C3=A1draig_Brady?= Content-Type: multipart/mixed; boundary=089e0111beb0942793052fab3ebb X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 23153 Cc: 23153@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --089e0111beb0942793052fab3ebb Content-Type: multipart/alternative; boundary=089e0111beb094278e052fab3eb9 --089e0111beb094278e052fab3eb9 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hello, Call to getenv(), which is in gnulib's backupfile.c, for cp, mv, ln and install, is done only if simple_backup_suffix is tilde which would imply we don't have backup or suffix option chosen while issuing the command. Thus, only option remaining is to checking the environment. Sincerely, Rishabh On Sun, Apr 3, 2016 at 2:33 PM, P=C3=A1draig Brady wrote= : > 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 =3D 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 --suffi= x >> 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=3Dnumbered= ' it >> becomes impossible to have suffixed backups (using --suffix or -b) until >> we >> do '--backup=3Dsimple' 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=C3=A1draig > --089e0111beb094278e052fab3eb9 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hello,

Call to getenv(), which= is in gnulib's backupfile.c, for cp, mv, ln and install, is done only = if simple_backup_suffix is tilde which would imply we don't have backup= or suffix option chosen while issuing the command. Thus, only option remai= ning is to checking the environment.

Sincerely,
Risha= bh

On Su= n, Apr 3, 2016 at 2:33 PM, P=C3=A1draig Brady <P@draigbrady.com> wrote:
On 29/03/16 1= 6: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
=C2=A0 =C2=A0 =C2=A0 we'll actually use backup_suffix_string.=C2=A0 */<= br> =C2=A0 =C2=A0backup_suffix_string =3D getenv ("SIMPLE_BACKUP_SUFFIX&qu= ot;);

Since we use backup_suffix_string to duplicate it into
simple_backup_suffix, I brought the getenv() call there, that too, only if<= br> required (as simple_backup_suffix already stores tilde already).


I did 'diff -ur'=C2=A0 directly against original cp.c (named cp-ori= ginal.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=3Dnumbere= d' it
becomes impossible to have suffixed backups (using --suffix or -b) until we=
do '--backup=3Dsimple' 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 s= ure.)



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


thanks,
P=C3=A1draig

--089e0111beb094278e052fab3eb9-- --089e0111beb0942793052fab3ebb Content-Type: text/x-patch; charset=US-ASCII; name="FIXME-regarding-getenv.patch" Content-Disposition: attachment; filename="FIXME-regarding-getenv.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_imm7kjlr0 ZGlmZiAtdXIgT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvbGliL2JhY2t1cGZpbGUuYyBEZWJ1Z2dp bmcvY29yZXV0aWxzLTguMjUvbGliL2JhY2t1cGZpbGUuYwotLS0gT3JpZ2luYWwvY29yZXV0aWxz LTguMjUvbGliL2JhY2t1cGZpbGUuYwkyMDE2LTAxLTAxIDE5OjE1OjU1LjAwMDAwMDAwMCArMDUz MAorKysgRGVidWdnaW5nL2NvcmV1dGlscy04LjI1L2xpYi9iYWNrdXBmaWxlLmMJMjAxNi0wNC0w MiAxODo0Njo1MS43NTY4ODgwMDAgKzA1MzAKQEAgLTI2NSw5ICsyNjUsMTYgQEAKIHsKICAgc2l6 ZV90IGZpbGVsZW4gPSBzdHJsZW4gKGZpbGUpOwogICBjaGFyICpzOworICBjaGFyICpzdWZmaXhf ZnJvbV9lbnY7CiAgIHNpemVfdCBzc2l6ZTsKICAgYm9vbCBzaW1wbGUgPSB0cnVlOwogCisKKyAg LyogSWYgc2ltcGxlX2JhY2t1cF9zdWZmaXggaXMgJ34nLCBjaGVjayBlbnZpcm9ubWVudCBpZiB3 ZSBoYXZlIGFueSB0aGVyZS4gKi8KKyAgaWYgKHN0cmNtcChzaW1wbGVfYmFja3VwX3N1ZmZpeCwg In4iKSA9PSAwKQorICAgIGlmICgoc3VmZml4X2Zyb21fZW52ID0gZ2V0ZW52KCJTSU1QTEVfQkFD S1VQX1NVRkZJWCIpKSAhPSBOVUxMKQorICAgICAgc2ltcGxlX2JhY2t1cF9zdWZmaXggPSB4c3Ry ZHVwIChzdWZmaXhfZnJvbV9lbnYpOworCiAgIC8qIEFsbG93IHJvb20gZm9yIHNpbXBsZSBvciAi Ln5OfiIgYmFja3Vwcy4gIFRoZSBndWVzcyBtdXN0IGJlIGF0CiAgICAgIGxlYXN0IHNpemVvZiAi Ln4xfiIsIGJ1dCBvdGhlcndpc2Ugd2lsbCBiZSBhZGp1c3RlZCBhcyBuZWVkZWQuICAqLwogICBz aXplX3Qgc2ltcGxlX2JhY2t1cF9zdWZmaXhfc2l6ZSA9IHN0cmxlbiAoc2ltcGxlX2JhY2t1cF9z dWZmaXgpICsgMTsKZGlmZiAtdXIgT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvc3JjL2NwLmMgRGVi dWdnaW5nL2NvcmV1dGlscy04LjI1L3NyYy9jcC5jCi0tLSBPcmlnaW5hbC9jb3JldXRpbHMtOC4y NS9zcmMvY3AuYwkyMDE2LTAxLTAxIDE5OjE4OjUwLjAwMDAwMDAwMCArMDUzMAorKysgRGVidWdn aW5nL2NvcmV1dGlscy04LjI1L3NyYy9jcC5jCTIwMTYtMDQtMDIgMTg6Mjk6MjAuMjg0ODg0MDAw ICswNTMwCkBAIC05MjAsNyArOTIwLDcgQEAKICAgaW50IGM7CiAgIGJvb2wgb2s7CiAgIGJvb2wg bWFrZV9iYWNrdXBzID0gZmFsc2U7Ci0gIGNoYXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5nOworICBj aGFyICpiYWNrdXBfc3VmZml4X3N0cmluZyA9IE5VTEw7CiAgIGNoYXIgKnZlcnNpb25fY29udHJv bF9zdHJpbmcgPSBOVUxMOwogICBzdHJ1Y3QgY3Bfb3B0aW9ucyB4OwogICBib29sIGNvcHlfY29u dGVudHMgPSBmYWxzZTsKQEAgLTkzOSwxMCArOTM5LDYgQEAKICAgc2VsaW51eF9lbmFibGVkID0g KDAgPCBpc19zZWxpbnV4X2VuYWJsZWQgKCkpOwogICBjcF9vcHRpb25faW5pdCAoJngpOwogCi0g IC8qIEZJWE1FOiBjb25zaWRlciBub3QgY2FsbGluZyBnZXRlbnYgZm9yIFNJTVBMRV9CQUNLVVBf U1VGRklYIHVubGVzcwotICAgICB3ZSdsbCBhY3R1YWxseSB1c2UgYmFja3VwX3N1ZmZpeF9zdHJp bmcuICAqLwotICBiYWNrdXBfc3VmZml4X3N0cmluZyA9IGdldGVudiAoIlNJTVBMRV9CQUNLVVBf U1VGRklYIik7Ci0KICAgd2hpbGUgKChjID0gZ2V0b3B0X2xvbmcgKGFyZ2MsIGFyZ3YsICJhYmRm SGlsTG5wcnN0OnV2eFBSUzpUWiIsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICBsb25nX29w dHMsIE5VTEwpKQogICAgICAgICAgIT0gLTEpCmRpZmYgLXVyIE9yaWdpbmFsL2NvcmV1dGlscy04 LjI1L3NyYy9pbnN0YWxsLmMgRGVidWdnaW5nL2NvcmV1dGlscy04LjI1L3NyYy9pbnN0YWxsLmMK LS0tIE9yaWdpbmFsL2NvcmV1dGlscy04LjI1L3NyYy9pbnN0YWxsLmMJMjAxNi0wMS0wMyAxODoy OTo0NC4wMDAwMDAwMDAgKzA1MzAKKysrIERlYnVnZ2luZy9jb3JldXRpbHMtOC4yNS9zcmMvaW5z dGFsbC5jCTIwMTYtMDQtMDIgMTg6MzE6MzYuODU2ODg0MDAwICswNTMwCkBAIC03ODQsNyArNzg0 LDcgQEAKICAgaW50IGV4aXRfc3RhdHVzID0gRVhJVF9TVUNDRVNTOwogICBjb25zdCBjaGFyICpz cGVjaWZpZWRfbW9kZSA9IE5VTEw7CiAgIGJvb2wgbWFrZV9iYWNrdXBzID0gZmFsc2U7Ci0gIGNo YXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5nOworICBjaGFyICpiYWNrdXBfc3VmZml4X3N0cmluZyA9 IE5VTEw7CiAgIGNoYXIgKnZlcnNpb25fY29udHJvbF9zdHJpbmcgPSBOVUxMOwogICBib29sIG1r ZGlyX2FuZF9pbnN0YWxsID0gZmFsc2U7CiAgIHN0cnVjdCBjcF9vcHRpb25zIHg7CkBAIC04MTMs MTAgKzgxMyw2IEBACiAgIGRpcl9hcmcgPSBmYWxzZTsKICAgdW1hc2sgKDApOwogCi0gIC8qIEZJ WE1FOiBjb25zaWRlciBub3QgY2FsbGluZyBnZXRlbnYgZm9yIFNJTVBMRV9CQUNLVVBfU1VGRklY IHVubGVzcwotICAgICB3ZSdsbCBhY3R1YWxseSB1c2UgYmFja3VwX3N1ZmZpeF9zdHJpbmcuICAq LwotICBiYWNrdXBfc3VmZml4X3N0cmluZyA9IGdldGVudiAoIlNJTVBMRV9CQUNLVVBfU1VGRklY Iik7Ci0KICAgd2hpbGUgKChvcHRjID0gZ2V0b3B0X2xvbmcgKGFyZ2MsIGFyZ3YsICJiY0NzRGRn Om06bzpwdDpUdlM6WiIsIGxvbmdfb3B0aW9ucywKICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIE5VTEwpKSAhPSAtMSkKICAgICB7CmRpZmYgLXVyIE9yaWdpbmFsL2NvcmV1dGlscy04LjI1 L3NyYy9sbi5jIERlYnVnZ2luZy9jb3JldXRpbHMtOC4yNS9zcmMvbG4uYwotLS0gT3JpZ2luYWwv Y29yZXV0aWxzLTguMjUvc3JjL2xuLmMJMjAxNi0wMS0wMSAxOToxODo1MC4wMDAwMDAwMDAgKzA1 MzAKKysrIERlYnVnZ2luZy9jb3JldXRpbHMtOC4yNS9zcmMvbG4uYwkyMDE2LTA0LTAyIDE4OjMx OjExLjc2NDg4NDAwMCArMDUzMApAQCAtNDY0LDcgKzQ2NCw3IEBACiAgIGludCBjOwogICBib29s IG9rOwogICBib29sIG1ha2VfYmFja3VwcyA9IGZhbHNlOwotICBjaGFyICpiYWNrdXBfc3VmZml4 X3N0cmluZzsKKyAgY2hhciAqYmFja3VwX3N1ZmZpeF9zdHJpbmcgPSBOVUxMOwogICBjaGFyICp2 ZXJzaW9uX2NvbnRyb2xfc3RyaW5nID0gTlVMTDsKICAgY2hhciBjb25zdCAqdGFyZ2V0X2RpcmVj dG9yeSA9IE5VTEw7CiAgIGJvb2wgbm9fdGFyZ2V0X2RpcmVjdG9yeSA9IGZhbHNlOwpAQCAtNDc5 LDEwICs0NzksNiBAQAogCiAgIGF0ZXhpdCAoY2xvc2Vfc3RkaW4pOwogCi0gIC8qIEZJWE1FOiBj b25zaWRlciBub3QgY2FsbGluZyBnZXRlbnYgZm9yIFNJTVBMRV9CQUNLVVBfU1VGRklYIHVubGVz cwotICAgICB3ZSdsbCBhY3R1YWxseSB1c2UgYmFja3VwX3N1ZmZpeF9zdHJpbmcuICAqLwotICBi YWNrdXBfc3VmZml4X3N0cmluZyA9IGdldGVudiAoIlNJTVBMRV9CQUNLVVBfU1VGRklYIik7Ci0K ICAgc3ltYm9saWNfbGluayA9IHJlbW92ZV9leGlzdGluZ19maWxlcyA9IGludGVyYWN0aXZlID0g dmVyYm9zZQogICAgID0gaGFyZF9kaXJfbGluayA9IGZhbHNlOwogCmRpZmYgLXVyIE9yaWdpbmFs L2NvcmV1dGlscy04LjI1L3NyYy9tdi5jIERlYnVnZ2luZy9jb3JldXRpbHMtOC4yNS9zcmMvbXYu YwotLS0gT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvc3JjL212LmMJMjAxNi0wMS0xMiAxNzoxMTo0 NC4wMDAwMDAwMDAgKzA1MzAKKysrIERlYnVnZ2luZy9jb3JldXRpbHMtOC4yNS9zcmMvbXYuYwky MDE2LTA0LTAyIDE4OjMwOjI0LjgzMjg4NDAwMCArMDUzMApAQCAtMzQ2LDcgKzM0Niw3IEBACiAg IGludCBjOwogICBib29sIG9rOwogICBib29sIG1ha2VfYmFja3VwcyA9IGZhbHNlOwotICBjaGFy ICpiYWNrdXBfc3VmZml4X3N0cmluZzsKKyAgY2hhciAqYmFja3VwX3N1ZmZpeF9zdHJpbmcgPSBO VUxMOwogICBjaGFyICp2ZXJzaW9uX2NvbnRyb2xfc3RyaW5nID0gTlVMTDsKICAgc3RydWN0IGNw X29wdGlvbnMgeDsKICAgY2hhciAqdGFyZ2V0X2RpcmVjdG9yeSA9IE5VTEw7CkBAIC0zNjgsMTAg KzM2OCw2IEBACiAgIC8qIFRyeSB0byBkaXNhYmxlIHRoZSBhYmlsaXR5IHRvIHVubGluayBhIGRp cmVjdG9yeS4gICovCiAgIHByaXZfc2V0X3JlbW92ZV9saW5rZGlyICgpOwogCi0gIC8qIEZJWE1F OiBjb25zaWRlciBub3QgY2FsbGluZyBnZXRlbnYgZm9yIFNJTVBMRV9CQUNLVVBfU1VGRklYIHVu bGVzcwotICAgICB3ZSdsbCBhY3R1YWxseSB1c2UgYmFja3VwX3N1ZmZpeF9zdHJpbmcuICAqLwot ICBiYWNrdXBfc3VmZml4X3N0cmluZyA9IGdldGVudiAoIlNJTVBMRV9CQUNLVVBfU1VGRklYIik7 Ci0KICAgd2hpbGUgKChjID0gZ2V0b3B0X2xvbmcgKGFyZ2MsIGFyZ3YsICJiZmludDp1dlM6VFoi LCBsb25nX29wdGlvbnMsIE5VTEwpKQogICAgICAgICAgIT0gLTEpCiAgICAgewo= --089e0111beb0942793052fab3ebb-- From debbugs-submit-bounces@debbugs.gnu.org Mon Apr 04 13:06:47 2016 Received: (at 23153) by debbugs.gnu.org; 4 Apr 2016 17:06:47 +0000 Received: from localhost ([127.0.0.1]:51332 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1an7xf-0006W2-BN for submit@debbugs.gnu.org; Mon, 04 Apr 2016 13:06:47 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:37375) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1an7xe-0006Vp-Cf for 23153@debbugs.gnu.org; Mon, 04 Apr 2016 13:06:46 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 1D85B161249; Mon, 4 Apr 2016 10:06:41 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 1p_fQs_K4jQN; Mon, 4 Apr 2016 10:06:40 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 62CB0161251; Mon, 4 Apr 2016 10:06:40 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id pGxsafmvTuEK; Mon, 4 Apr 2016 10:06:40 -0700 (PDT) Received: from penguin.cs.ucla.edu (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 46B11161249; Mon, 4 Apr 2016 10:06:40 -0700 (PDT) Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c To: Rishabh Dave , =?UTF-8?Q?P=c3=a1draig_Brady?= References: <5700DC47.7080405@draigBrady.com> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: <57029F20.4050608@cs.ucla.edu> Date: Mon, 4 Apr 2016 10:06:40 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -1.0 (-) X-Debbugs-Envelope-To: 23153 Cc: 23153@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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. From debbugs-submit-bounces@debbugs.gnu.org Tue Apr 05 10:49:32 2016 Received: (at 23153) by debbugs.gnu.org; 5 Apr 2016 14:49:32 +0000 Received: from localhost ([127.0.0.1]:52437 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1anSIO-00044i-DM for submit@debbugs.gnu.org; Tue, 05 Apr 2016 10:49:32 -0400 Received: from mail-oi0-f42.google.com ([209.85.218.42]:33380) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1anSIM-00044T-7G for 23153@debbugs.gnu.org; Tue, 05 Apr 2016 10:49:30 -0400 Received: by mail-oi0-f42.google.com with SMTP id w85so20331150oiw.0 for <23153@debbugs.gnu.org>; Tue, 05 Apr 2016 07:49:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=CpY93+rZOnBLcwW7suaU3xuJFVstGn5XxdnCFcwUo4A=; b=zNe714VOYqJGasxNduUB82v8HiV+ggIFq4AptXOlE9d4AftgEeqeZUp9uB+iVLXTQF Uzf2Fscv3J3sAgIAkHrNZGCFl11AiSXJMVw1/wsPgSC6ngqXJdjXF6d/Ij2OXAEP/o+K 1k7fXtOydh4UxyYgEMIIopytoLbi051TQBKtWlynE81zumotBJL1Km//P7uxpPj4oDl6 Y87Sr52N1iE1RcSprwp5Jt61tfmo7pMkgV73HJ2wX4H2Lz7lS08B5FYAwO493o9/LxQZ O7otTtJL5wgbS2vdE5xBqhJHIHW4ULkd/rCfRppOZ2aGW7mp0MlY9paieHe1cXMrGn02 AcVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=CpY93+rZOnBLcwW7suaU3xuJFVstGn5XxdnCFcwUo4A=; b=Octsvx0wTMMAD3md9L+XfqBvZoFhZKIyl+cGWMI6Kb7jAaITFXJZrxFipHLlkgoAKO sKHeP/zgF69FOd6kP3mx9vahUKrl/JC/Zd+jagskEzmdaWsry6aOShtmG4Jq7rNWeK8h ZwhWSUkaTvB8LAEkD7VBL6/5Rk1fcN7kL8ZOBWoIvaNyWNg+zhR3HK07YE+ugDBi4t7X TvlvJFeUjraDUdA1MB1nF1nHpFcf33mT0K/MfdVMcXmGvoNVlyrKpNUrThDnBGYCYxTR 6gLhjvVy6xjx/yyy4fprekwzxNNGnvGK/fr+5mIUc6hcpJDv05WkCwTAplRyPqxphO/G GAoA== X-Gm-Message-State: AD7BkJLEV9xTAuIKt8VNUgE0EDHGeYJ4a2M9J2jXraYqOHbPUPZnmx7Ixqd9q+RA2C9tnVMbMZK1U3/AARq6PA== MIME-Version: 1.0 X-Received: by 10.157.63.116 with SMTP id m107mr13393625otc.115.1459867764566; Tue, 05 Apr 2016 07:49:24 -0700 (PDT) Received: by 10.60.141.101 with HTTP; Tue, 5 Apr 2016 07:49:24 -0700 (PDT) In-Reply-To: <57029F20.4050608@cs.ucla.edu> References: <5700DC47.7080405@draigBrady.com> <57029F20.4050608@cs.ucla.edu> Date: Tue, 5 Apr 2016 20:19:24 +0530 Message-ID: Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c From: Rishabh Dave To: Paul Eggert Content-Type: multipart/mixed; boundary=001a1147138086fa15052fbdf384 X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 23153 Cc: 23153@debbugs.gnu.org, =?UTF-8?Q?P=C3=A1draig_Brady?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --001a1147138086fa15052fbdf384 Content-Type: multipart/alternative; boundary=001a1147138086fa11052fbdf382 --001a1147138086fa11052fbdf382 Content-Type: text/plain; charset=UTF-8 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 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. > --001a1147138086fa11052fbdf382 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Did fix that - changing function call to GNU sty= le and "suffix_from_env" to local variable. The corresponding pat= ch is attached.

On Mon, Apr 4, 2016 at 10:36 PM, Paul Eggert <eggert= @cs.ucla.edu> wrote:
On 04/= 04/2016 09:30 AM, Rishabh Dave wrote:
+=C2=A0 char *suffix_from_env;
=C2=A0 =C2=A0 size_t ssize;
=C2=A0 =C2=A0 bool simple =3D true;
=C2=A0 +
+=C2=A0 /* If simple_backup_suffix is '~', check environment if we = have any there. */
+=C2=A0 if (strcmp(simple_backup_suffix, "~") =3D=3D 0)
+=C2=A0 =C2=A0 if ((suffix_from_env =3D getenv("SIMPLE_BACKUP_SUFFIX&q= uot;)) !=3D NULL)
+=C2=A0 =C2=A0 =C2=A0 simple_backup_suffix =3D xstrdup (suffix_from_env); +

The usual GNU style is "f (x)" rather than "f(x)" for f= unction calls. Also, avoid side effects in an "if"; just have the= then-part with a local variable "suffix_from_env" and put the su= bsidary if after that.

--001a1147138086fa11052fbdf382-- --001a1147138086fa15052fbdf384 Content-Type: text/x-patch; charset=US-ASCII; name="FIXME-regarding-getenv.patch" Content-Disposition: attachment; filename="FIXME-regarding-getenv.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_imnjdfoo0 ZGlmZiAtdXIgT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvbGliL2JhY2t1cGZpbGUuYyBEZWJ1Z2dp bmcvY29yZXV0aWxzLTguMjUvbGliL2JhY2t1cGZpbGUuYwotLS0gT3JpZ2luYWwvY29yZXV0aWxz LTguMjUvbGliL2JhY2t1cGZpbGUuYwkyMDE2LTAxLTAxIDE5OjE1OjU1LjAwMDAwMDAwMCArMDUz MAorKysgRGVidWdnaW5nL2NvcmV1dGlscy04LjI1L2xpYi9iYWNrdXBmaWxlLmMJMjAxNi0wNC0w NSAyMDoxMToxMi4wOTY1NjgyNzggKzA1MzAKQEAgLTI2OCw2ICsyNjgsMTUgQEAKICAgc2l6ZV90 IHNzaXplOwogICBib29sIHNpbXBsZSA9IHRydWU7CiAKKworICAvKiBJZiBzaW1wbGVfYmFja3Vw X3N1ZmZpeCBpcyAnficsIGNoZWNrIGVudmlyb25tZW50IGlmIHdlIGhhdmUgYW55IHRoZXJlLiAq LworICBpZiAoc3RyY21wIChzaW1wbGVfYmFja3VwX3N1ZmZpeCwgIn4iKSA9PSAwKQorICAgIHsK KyAgICAgIGNoYXIgKnN1ZmZpeF9mcm9tX2VudjsKKyAgICAgIGlmICgoc3VmZml4X2Zyb21fZW52 ID0gZ2V0ZW52ICgiU0lNUExFX0JBQ0tVUF9TVUZGSVgiKSkgIT0gTlVMTCkgCisgICAgICAgIHNp bXBsZV9iYWNrdXBfc3VmZml4ID0geHN0cmR1cCAoc3VmZml4X2Zyb21fZW52KTsKKyAgICB9CisK ICAgLyogQWxsb3cgcm9vbSBmb3Igc2ltcGxlIG9yICIufk5+IiBiYWNrdXBzLiAgVGhlIGd1ZXNz IG11c3QgYmUgYXQKICAgICAgbGVhc3Qgc2l6ZW9mICIufjF+IiwgYnV0IG90aGVyd2lzZSB3aWxs IGJlIGFkanVzdGVkIGFzIG5lZWRlZC4gICovCiAgIHNpemVfdCBzaW1wbGVfYmFja3VwX3N1ZmZp eF9zaXplID0gc3RybGVuIChzaW1wbGVfYmFja3VwX3N1ZmZpeCkgKyAxOwpkaWZmIC11ciBPcmln aW5hbC9jb3JldXRpbHMtOC4yNS9zcmMvY3AuYyBEZWJ1Z2dpbmcvY29yZXV0aWxzLTguMjUvc3Jj L2NwLmMKLS0tIE9yaWdpbmFsL2NvcmV1dGlscy04LjI1L3NyYy9jcC5jCTIwMTYtMDEtMDEgMTk6 MTg6NTAuMDAwMDAwMDAwICswNTMwCisrKyBEZWJ1Z2dpbmcvY29yZXV0aWxzLTguMjUvc3JjL2Nw LmMJMjAxNi0wNC0wMiAxODoyOToyMC4yODQ4ODQwMDAgKzA1MzAKQEAgLTkyMCw3ICs5MjAsNyBA QAogICBpbnQgYzsKICAgYm9vbCBvazsKICAgYm9vbCBtYWtlX2JhY2t1cHMgPSBmYWxzZTsKLSAg Y2hhciAqYmFja3VwX3N1ZmZpeF9zdHJpbmc7CisgIGNoYXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5n ID0gTlVMTDsKICAgY2hhciAqdmVyc2lvbl9jb250cm9sX3N0cmluZyA9IE5VTEw7CiAgIHN0cnVj dCBjcF9vcHRpb25zIHg7CiAgIGJvb2wgY29weV9jb250ZW50cyA9IGZhbHNlOwpAQCAtOTM5LDEw ICs5MzksNiBAQAogICBzZWxpbnV4X2VuYWJsZWQgPSAoMCA8IGlzX3NlbGludXhfZW5hYmxlZCAo KSk7CiAgIGNwX29wdGlvbl9pbml0ICgmeCk7CiAKLSAgLyogRklYTUU6IGNvbnNpZGVyIG5vdCBj YWxsaW5nIGdldGVudiBmb3IgU0lNUExFX0JBQ0tVUF9TVUZGSVggdW5sZXNzCi0gICAgIHdlJ2xs IGFjdHVhbGx5IHVzZSBiYWNrdXBfc3VmZml4X3N0cmluZy4gICovCi0gIGJhY2t1cF9zdWZmaXhf c3RyaW5nID0gZ2V0ZW52ICgiU0lNUExFX0JBQ0tVUF9TVUZGSVgiKTsKLQogICB3aGlsZSAoKGMg PSBnZXRvcHRfbG9uZyAoYXJnYywgYXJndiwgImFiZGZIaWxMbnByc3Q6dXZ4UFJTOlRaIiwKICAg ICAgICAgICAgICAgICAgICAgICAgICAgIGxvbmdfb3B0cywgTlVMTCkpCiAgICAgICAgICAhPSAt MSkKZGlmZiAtdXIgT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvc3JjL2luc3RhbGwuYyBEZWJ1Z2dp bmcvY29yZXV0aWxzLTguMjUvc3JjL2luc3RhbGwuYwotLS0gT3JpZ2luYWwvY29yZXV0aWxzLTgu MjUvc3JjL2luc3RhbGwuYwkyMDE2LTAxLTAzIDE4OjI5OjQ0LjAwMDAwMDAwMCArMDUzMAorKysg RGVidWdnaW5nL2NvcmV1dGlscy04LjI1L3NyYy9pbnN0YWxsLmMJMjAxNi0wNC0wMiAxODozMToz Ni44NTY4ODQwMDAgKzA1MzAKQEAgLTc4NCw3ICs3ODQsNyBAQAogICBpbnQgZXhpdF9zdGF0dXMg PSBFWElUX1NVQ0NFU1M7CiAgIGNvbnN0IGNoYXIgKnNwZWNpZmllZF9tb2RlID0gTlVMTDsKICAg Ym9vbCBtYWtlX2JhY2t1cHMgPSBmYWxzZTsKLSAgY2hhciAqYmFja3VwX3N1ZmZpeF9zdHJpbmc7 CisgIGNoYXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5nID0gTlVMTDsKICAgY2hhciAqdmVyc2lvbl9j b250cm9sX3N0cmluZyA9IE5VTEw7CiAgIGJvb2wgbWtkaXJfYW5kX2luc3RhbGwgPSBmYWxzZTsK ICAgc3RydWN0IGNwX29wdGlvbnMgeDsKQEAgLTgxMywxMCArODEzLDYgQEAKICAgZGlyX2FyZyA9 IGZhbHNlOwogICB1bWFzayAoMCk7CiAKLSAgLyogRklYTUU6IGNvbnNpZGVyIG5vdCBjYWxsaW5n IGdldGVudiBmb3IgU0lNUExFX0JBQ0tVUF9TVUZGSVggdW5sZXNzCi0gICAgIHdlJ2xsIGFjdHVh bGx5IHVzZSBiYWNrdXBfc3VmZml4X3N0cmluZy4gICovCi0gIGJhY2t1cF9zdWZmaXhfc3RyaW5n ID0gZ2V0ZW52ICgiU0lNUExFX0JBQ0tVUF9TVUZGSVgiKTsKLQogICB3aGlsZSAoKG9wdGMgPSBn ZXRvcHRfbG9uZyAoYXJnYywgYXJndiwgImJjQ3NEZGc6bTpvOnB0OlR2UzpaIiwgbG9uZ19vcHRp b25zLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgTlVMTCkpICE9IC0xKQogICAgIHsK ZGlmZiAtdXIgT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvc3JjL2xuLmMgRGVidWdnaW5nL2NvcmV1 dGlscy04LjI1L3NyYy9sbi5jCi0tLSBPcmlnaW5hbC9jb3JldXRpbHMtOC4yNS9zcmMvbG4uYwky MDE2LTAxLTAxIDE5OjE4OjUwLjAwMDAwMDAwMCArMDUzMAorKysgRGVidWdnaW5nL2NvcmV1dGls cy04LjI1L3NyYy9sbi5jCTIwMTYtMDQtMDIgMTg6MzE6MTEuNzY0ODg0MDAwICswNTMwCkBAIC00 NjQsNyArNDY0LDcgQEAKICAgaW50IGM7CiAgIGJvb2wgb2s7CiAgIGJvb2wgbWFrZV9iYWNrdXBz ID0gZmFsc2U7Ci0gIGNoYXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5nOworICBjaGFyICpiYWNrdXBf c3VmZml4X3N0cmluZyA9IE5VTEw7CiAgIGNoYXIgKnZlcnNpb25fY29udHJvbF9zdHJpbmcgPSBO VUxMOwogICBjaGFyIGNvbnN0ICp0YXJnZXRfZGlyZWN0b3J5ID0gTlVMTDsKICAgYm9vbCBub190 YXJnZXRfZGlyZWN0b3J5ID0gZmFsc2U7CkBAIC00NzksMTAgKzQ3OSw2IEBACiAKICAgYXRleGl0 IChjbG9zZV9zdGRpbik7CiAKLSAgLyogRklYTUU6IGNvbnNpZGVyIG5vdCBjYWxsaW5nIGdldGVu diBmb3IgU0lNUExFX0JBQ0tVUF9TVUZGSVggdW5sZXNzCi0gICAgIHdlJ2xsIGFjdHVhbGx5IHVz ZSBiYWNrdXBfc3VmZml4X3N0cmluZy4gICovCi0gIGJhY2t1cF9zdWZmaXhfc3RyaW5nID0gZ2V0 ZW52ICgiU0lNUExFX0JBQ0tVUF9TVUZGSVgiKTsKLQogICBzeW1ib2xpY19saW5rID0gcmVtb3Zl X2V4aXN0aW5nX2ZpbGVzID0gaW50ZXJhY3RpdmUgPSB2ZXJib3NlCiAgICAgPSBoYXJkX2Rpcl9s aW5rID0gZmFsc2U7CiAKZGlmZiAtdXIgT3JpZ2luYWwvY29yZXV0aWxzLTguMjUvc3JjL212LmMg RGVidWdnaW5nL2NvcmV1dGlscy04LjI1L3NyYy9tdi5jCi0tLSBPcmlnaW5hbC9jb3JldXRpbHMt OC4yNS9zcmMvbXYuYwkyMDE2LTAxLTEyIDE3OjExOjQ0LjAwMDAwMDAwMCArMDUzMAorKysgRGVi dWdnaW5nL2NvcmV1dGlscy04LjI1L3NyYy9tdi5jCTIwMTYtMDQtMDIgMTg6MzA6MjQuODMyODg0 MDAwICswNTMwCkBAIC0zNDYsNyArMzQ2LDcgQEAKICAgaW50IGM7CiAgIGJvb2wgb2s7CiAgIGJv b2wgbWFrZV9iYWNrdXBzID0gZmFsc2U7Ci0gIGNoYXIgKmJhY2t1cF9zdWZmaXhfc3RyaW5nOwor ICBjaGFyICpiYWNrdXBfc3VmZml4X3N0cmluZyA9IE5VTEw7CiAgIGNoYXIgKnZlcnNpb25fY29u dHJvbF9zdHJpbmcgPSBOVUxMOwogICBzdHJ1Y3QgY3Bfb3B0aW9ucyB4OwogICBjaGFyICp0YXJn ZXRfZGlyZWN0b3J5ID0gTlVMTDsKQEAgLTM2OCwxMCArMzY4LDYgQEAKICAgLyogVHJ5IHRvIGRp c2FibGUgdGhlIGFiaWxpdHkgdG8gdW5saW5rIGEgZGlyZWN0b3J5LiAgKi8KICAgcHJpdl9zZXRf cmVtb3ZlX2xpbmtkaXIgKCk7CiAKLSAgLyogRklYTUU6IGNvbnNpZGVyIG5vdCBjYWxsaW5nIGdl dGVudiBmb3IgU0lNUExFX0JBQ0tVUF9TVUZGSVggdW5sZXNzCi0gICAgIHdlJ2xsIGFjdHVhbGx5 IHVzZSBiYWNrdXBfc3VmZml4X3N0cmluZy4gICovCi0gIGJhY2t1cF9zdWZmaXhfc3RyaW5nID0g Z2V0ZW52ICgiU0lNUExFX0JBQ0tVUF9TVUZGSVgiKTsKLQogICB3aGlsZSAoKGMgPSBnZXRvcHRf bG9uZyAoYXJnYywgYXJndiwgImJmaW50OnV2UzpUWiIsIGxvbmdfb3B0aW9ucywgTlVMTCkpCiAg ICAgICAgICAhPSAtMSkKICAgICB7Cg== --001a1147138086fa15052fbdf384-- From debbugs-submit-bounces@debbugs.gnu.org Wed Nov 02 13:59:40 2016 Received: (at 23153) by debbugs.gnu.org; 2 Nov 2016 17:59:40 +0000 Received: from localhost ([127.0.0.1]:40852 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c1zp5-0004mJ-VD for submit@debbugs.gnu.org; Wed, 02 Nov 2016 13:59:40 -0400 Received: from mail.magicbluesmoke.com ([82.195.144.49]:46152) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c1zp4-0004m9-EV for 23153@debbugs.gnu.org; Wed, 02 Nov 2016 13:59:38 -0400 Received: from [192.168.1.80] (unknown [109.79.133.123]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.magicbluesmoke.com (Postfix) with ESMTPSA id 11F68F7; Wed, 2 Nov 2016 17:59:35 +0000 (GMT) Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c To: Rishabh Dave , bug-gnulib References: <5700DC47.7080405@draigBrady.com> <57029F20.4050608@cs.ucla.edu> From: =?UTF-8?Q?P=c3=a1draig_Brady?= Message-ID: <22e6eb7e-66df-5dd6-7129-76adc9d46fe7@draigBrady.com> Date: Wed, 2 Nov 2016 17:59:34 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------8237ED89FB5D53BEB94B1168" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 23153 Cc: 23153@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) This is a multi-part message in MIME format. --------------8237ED89FB5D53BEB94B1168 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit unarchive 23153 stop On 05/04/16 15:49, Rishabh Dave wrote: > 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 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); >>> + Sorry for the delay. I'll apply the attached variant to gnulib instead. That has the property that --suffix=~ will still override an env variable of SIMPLE_BACKUP_SUFFIX. It also avoids a redundant malloc, and handles empty env variables. I'll then apply the coreutils patches in your name. thanks, Pádraig --------------8237ED89FB5D53BEB94B1168 Content-Type: text/x-patch; name="gnulib-backupfile-no-getenv.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gnulib-backupfile-no-getenv.patch" >From b6bbddad76b64001f05b95b2f18dc40e809354d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Wed, 2 Nov 2016 17:52:12 +0000 Subject: [PATCH] backupfile: initialize default suffix within the implementation * lib/backupfile.c (find_backup_file_name): Initialize the global variable here, to simplify usage, and to only call getenv() when needed. --- lib/backupfile.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/backupfile.c b/lib/backupfile.c index 1fe369e..291faf5 100644 --- a/lib/backupfile.c +++ b/lib/backupfile.c @@ -80,7 +80,7 @@ /* The extension added to file names to produce a simple (as opposed to numbered) backup file name. */ -char const *simple_backup_suffix = "~"; +char const *simple_backup_suffix = NULL; /* If FILE (which was of length FILELEN before an extension was @@ -268,6 +268,16 @@ find_backup_file_name (char const *file, enum backup_type backup_type) size_t ssize; bool simple = true; + /* Initialize the default simple backup suffix. */ + if (! simple_backup_suffix) + { + char const* env_suffix = getenv ("SIMPLE_BACKUP_SUFFIX"); + if (env_suffix && *env_suffix) + simple_backup_suffix = env_suffix; + else + simple_backup_suffix = "~"; + } + /* 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; -- 2.5.5 --------------8237ED89FB5D53BEB94B1168-- From debbugs-submit-bounces@debbugs.gnu.org Wed Nov 02 20:06:46 2016 Received: (at 23153-done) by debbugs.gnu.org; 3 Nov 2016 00:06:46 +0000 Received: from localhost ([127.0.0.1]:40981 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c25YL-0006Sj-Ta for submit@debbugs.gnu.org; Wed, 02 Nov 2016 20:06:46 -0400 Received: from mail.magicbluesmoke.com ([82.195.144.49]:47458) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c25YJ-0006Sa-Mo for 23153-done@debbugs.gnu.org; Wed, 02 Nov 2016 20:06:44 -0400 Received: from [192.168.1.80] (unknown [109.79.133.123]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.magicbluesmoke.com (Postfix) with ESMTPSA id 68E14949C; Thu, 3 Nov 2016 00:06:42 +0000 (GMT) Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c To: Rishabh Dave References: <5700DC47.7080405@draigBrady.com> <57029F20.4050608@cs.ucla.edu> From: =?UTF-8?Q?P=c3=a1draig_Brady?= Message-ID: <2a04bda5-c6ee-b372-9feb-e3dc9e0c1f16@draigBrady.com> Date: Thu, 3 Nov 2016 00:06:41 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------BE74DFAD4A1B869755FB25C1" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 23153-done Cc: 23153-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) This is a multi-part message in MIME format. --------------BE74DFAD4A1B869755FB25C1 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Attached are the two patches I intend to push for this upon the next gnulib update. Marking this bug as done. thanks, Pádraig --------------BE74DFAD4A1B869755FB25C1 Content-Type: text/x-patch; name="backupfile-cleanup.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="backupfile-cleanup.patch" >From 2535447f85c4ca5b7caea0bf144e125267e0fa95 Mon Sep 17 00:00:00 2001 From: Rishabh Dave 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?= 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 --------------BE74DFAD4A1B869755FB25C1-- From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 03 18:44:18 2016 Received: (at 23153) by debbugs.gnu.org; 3 Nov 2016 22:44:18 +0000 Received: from localhost ([127.0.0.1]:41958 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c2Qk6-0000QM-Ho for submit@debbugs.gnu.org; Thu, 03 Nov 2016 18:44:18 -0400 Received: from mout.kundenserver.de ([212.227.17.13]:52372) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c2Qk5-0000Q8-6W for 23153@debbugs.gnu.org; Thu, 03 Nov 2016 18:44:17 -0400 Received: from [192.168.101.10] ([217.86.86.51]) by mrelayeu.kundenserver.de (mreue104) with ESMTPSA (Nemesis) id 0MabU1-1cHc6i3NKj-00K4lf; Thu, 03 Nov 2016 23:44:09 +0100 Subject: Re: bug#23153: [PATCH]: For FIXME in cp.c To: 23153@debbugs.gnu.org, P@draigBrady.com, rishabhddave@gmail.com References: <5700DC47.7080405@draigBrady.com> <57029F20.4050608@cs.ucla.edu> <2a04bda5-c6ee-b372-9feb-e3dc9e0c1f16@draigBrady.com> From: Bernhard Voelker Message-ID: <7530abde-452c-01e5-837f-c881a37ec50a@bernhard-voelker.de> Date: Thu, 3 Nov 2016 23:44:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <2a04bda5-c6ee-b372-9feb-e3dc9e0c1f16@draigBrady.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K0:I+UoaG5QP0ahgYFvIIKkELHkBJTe+FzOAdZKOojMrUWU2U1gKU7 sHdvoNbmaoQwTO512mYAn6ou4FTKtU6AzKJF9X8y6VS7yEDgxFZ3Aa79wEw0gpo/QFZ0FiW Oo7JhqjQ6Ta6vuHMLq4RhUF7MiuGUBL9XdXRqR8lJemRDoDQge8rvLsE0ZMRCEhIwzHN5gL nSTUzGgvcZ5nEQa7CuYEQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:nCUg90TxArw=:O7QEwmJgBL8G6JN53ZAmWl 0OyR84K6enVPN0sA/wa+Kq8Z2ckSh32BEru7TQFiRvCchRpOjn0WCKnHmIuGZrbxpLxIXuiBO +zOOgWGa+krYLq8cv0oVaQ/5rgB8hRrzYpx0RMXYlPi8BR25fFMeXiWKJkq2us2lIRUCwOoWP zYhpNY02hU9Rv45yer6V3SshTbWZXz9QxzN014dPfzCrKfyzfFnTDxKGwjn/YLG4/RYyrRQQp podIBlfhPkqUcGlNatwVUETcH4+MxfhmPk3r+7fTJXVxtB8c6fRK2CiULdvt1UEUZTfw1eL0g PT77T2puBOK33VyzrIZ2asbzddcY+5Sj9S2VV9XyGbqtBkU/ySLOE4eiC8cTR/rGD3LYBkSjm VlwDq/mqMSMNSq3I1AVQDf64a5Owor9zKq2mYyOQbi2leLlk0b19STJ+Lotb1ZUl5R4uEZxlj dYYYsIg2a4zXl1P6RZWkiVgrBNb8cuLiG76D20E+037x10Pypr/oX7qidgwmEiQYBzj7RjUq9 k8rLSIiDTF2+iWCRaVLHaGxAO0+mRsv3y2ORylddNYca4uerdTrshetUm3hn1KwaPSi1I3Bd/ oHyPJdIRaX3GInNUx8RkBJOfyaD0XjUqAAmWvm16VIz+e5wfgGr0y4n1iCxICUTWwZtRY3R2w SXZcfaUb0KnYqjk8uTgQg6E7udAr0tuqx5S3zC9UA+z4DksTEkhTvxtTcYRheGCi5FQCRuRsd UIsPnIBIpHzc+cC7 X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 23153 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.0 (/) On 11/03/2016 01:06 AM, Pádraig Brady wrote: > Subject: [PATCH 1/2] maint: simplify handling of backup --suffix in various > [...] > Subject: [PATCH 2/2] maint: refactor printing of backup suffix Nice one, thanks! Have a nice day, Berny From unknown Fri Sep 05 20:55:28 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Fri, 02 Dec 2016 12:24:03 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator