[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils-bug
Subject:    bug#15173: [cp] --link overrides dereference settings
From:       Gian Piero Carrubba <gpiero () rm-rf ! it>
Date:       2013-10-31 21:28:47
Message-ID: 20131031212847.GE30875 () caimano ! fdc ! rm-rf ! it
[Download RAW message or body]

* [Thu, Oct 31, 2013 at 02:54:59PM +0100] Gian Piero Carrubba:
>Oh... I was tweaking a bit the last patch posted by Bernhard in order 
>to let it be POSIX compliant (but now I have to add: "for my 
>interpretation of POSIX"), but didn't had the time to complete before 
>going to work.  Will probably finish it up anyway this evening or 
>tomorrow, at this point at least for comparing how far we're currently 
>are from "POSIX".

Here it is.
First of all, sorry for having generated a lot of noise. I completely 
misunderstood the meaning of --no-preserve=links. Having get rid of 
that, the differences in the results of the tests are a lot less.

$ fgrep "DIFFERENCE in Test: " testit.log | sed 's/^.*: //'
cp  -l   filelink ...
cp  -l  --preserve=links filelink ...
cp -L -l   filelink ...
cp -L -l  --preserve=links filelink ...
cp -L -l -R  filelink ...
cp -L -l -R --preserve=links filelink ...
cp -H -l   filelink ...
cp -H -l  --preserve=links filelink ...
cp -H -l -R  filelink ...
cp -H -l -R --preserve=links filelink ...

I think they are all legit (full log and differences summary attached).

* Changes with regard to the last patch posted by Bernhard.

=== Minor changes

- use 'dereference' instead of 'deref' in order to be consistent with 
   'x->dereference' and for passing the "grep test"[0]
- use 'command_line_arg' instead of 'cli_arg' in order to be consistent 
   with an already used variable and for passing the "grep test"
- move up the declaration of bool dereference in order to use it for 
   every invocation of create_hard_link()
- clean up the initialization of 'flags' in create_hard_link()

[0] i.e.: http://jamie-wong.com/2013/07/12/grep-test/

=== Major changes

- dereference by default (i.e.: unless --no-dereference is used) the 
   source files.
   I promise that I will not insist anymore :), but I really think this 
   is important for consistency from a user point of view.

Ciao,
Gian Piero.

["bug-15173.patch" (text/x-diff)]

Thu Oct 31 21:57:32 CET 2013  Gian Piero Carrubba <gpiero@rm-rf.it>
  * bug-15173
  http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15173
diff -rN -u old-trunk/NEWS new-trunk/NEWS
--- old-trunk/NEWS	2013-10-31 22:24:51.690232614 +0100
+++ new-trunk/NEWS	2013-10-31 22:24:51.694232641 +0100
@@ -70,6 +70,10 @@
 
 ** Changes in behavior
 
+  cp --link now dereferences a symbolic link as source files before creating
+  the hard link in the destination, unless --no-dereference is used too.
+  Previously, it would create a hard link of the symbolic link.
+
   dd status=none now suppresses all non fatal diagnostic messages,
   not just the transfer counts.
 
diff -rN -u old-trunk/src/copy.c new-trunk/src/copy.c
--- old-trunk/src/copy.c	2013-10-31 22:24:51.690232614 +0100
+++ new-trunk/src/copy.c	2013-10-31 22:24:51.698232676 +0100
@@ -1558,18 +1558,23 @@
            _("failed to restore the default file creation context"));
 }
 
-/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE and
-   VERBOSE settings.  Return true upon success.  Otherwise, diagnose
-   the failure and return false.
-   If SRC_NAME is a symbolic link it will not be followed.  If the system
-   doesn't support hard links to symbolic links, then DST_NAME will
-   be created as a symbolic link to SRC_NAME.  */
+/* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE, VERBOSE and
+   DEREFERENCE settings. Return true upon success. Otherwise, diagnose the
+   failure and return false. If SRC_NAME is a symbolic link it will not be
+   followed unless DEREFERENCE is true.
+   If the system doesn't support hard links to symbolic links, then DST_NAME
+   will be created as a symbolic link to SRC_NAME.  */
 static bool
 create_hard_link (char const *src_name, char const *dst_name,
-                  bool replace, bool verbose)
+                  bool replace, bool verbose, bool dereference)
 {
-  /* We want to guarantee that symlinks are not followed.  */
-  bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+  /* We want to guarantee that symlinks are not followed, unless requested. */
+  int flags = 0;
+  if (dereference)
+    flags = AT_SYMLINK_FOLLOW;
+
+  bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags)
+                      != 0);
 
   /* If the link failed because of an existing destination,
      remove that file and then call link again.  */
@@ -1582,7 +1587,8 @@
         }
       if (verbose)
         printf (_("removed %s\n"), quote (dst_name));
-      link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0);
+      link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags)
+                     != 0);
     }
 
   if (link_failed)
@@ -1595,6 +1601,17 @@
   return true;
 }
 
+/* Return true if the current file should be (tried to be) dereferenced:
+   either for DEREF_ALWAYS or for DEREF_COMMAND_LINE_ARGUMENTS in the case
+   where the current file is a COMMAND_LINE_ARG; otherwise return false. */
+static inline bool _GL_ATTRIBUTE_PURE
+should_dereference (const struct cp_options *x, bool command_line_arg)
+{
+  return x->dereference == DEREF_ALWAYS
+         || (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
+             && command_line_arg);
+}
+
 /* Copy the file SRC_NAME to the file DST_NAME.  The files may be of
    any type.  NEW_DST should be true if the file DST_NAME cannot
    exist because its parent directory was just created; NEW_DST should
@@ -1670,6 +1687,8 @@
       record_file (x->src_info, src_name, &src_sb);
     }
 
+  bool dereference = should_dereference (x, command_line_arg);
+
   if (!new_dst)
     {
       /* Regular files can be created by writing through symbolic
@@ -1748,7 +1767,7 @@
                       /* Note we currently replace DST_NAME unconditionally,
                          even if it was a newer separate file.  */
                       if (! create_hard_link (earlier_file, dst_name, true,
-                                              x->verbose))
+                                              x->verbose, dereference))
                         {
                           goto un_backup;
                         }
@@ -2078,7 +2097,8 @@
         }
       else
         {
-          if (! create_hard_link (earlier_file, dst_name, true, x->verbose))
+          if (! create_hard_link (earlier_file, dst_name, true, x->verbose,
+                                  dereference))
             goto un_backup;
 
           return true;
@@ -2389,7 +2409,7 @@
            && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
                 && x->dereference == DEREF_NEVER))
     {
-      if (! create_hard_link (src_name, dst_name, false, false))
+      if (! create_hard_link (src_name, dst_name, false, false, dereference))
         goto un_backup;
     }
   else if (S_ISREG (src_mode)
diff -rN -u old-trunk/tests/cp/same-file.sh new-trunk/tests/cp/same-file.sh
--- old-trunk/tests/cp/same-file.sh	2013-10-31 22:24:51.690232614 +0100
+++ new-trunk/tests/cp/same-file.sh	2013-10-31 22:24:51.698232676 +0100
@@ -189,9 +189,9 @@
 0 -bf (foo sl1 -> foo sl2 sl2.~1~ -> foo)
 0 -bdf (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
 1 -l [cp: cannot create hard link 'sl2' to 'sl1'] (foo sl1 -> foo sl2 -> foo)
-0 -fl (foo sl1 -> foo sl2 -> foo)
-0 -bl (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
-0 -bfl (foo sl1 -> foo sl2 -> foo sl2.~1~ -> foo)
+0 -fl (foo sl1 -> foo sl2)
+0 -bl (foo sl1 -> foo sl2 sl2.~1~ -> foo)
+0 -bfl (foo sl1 -> foo sl2 sl2.~1~ -> foo)
 
 1 [cp: 'foo' and 'hardlink' are the same file] (foo hardlink)
 1 -d [cp: 'foo' and 'hardlink' are the same file] (foo hardlink)

["testit-differences-summary.log.gz" (application/octet-stream)]
["testit.log.gz" (text/plain)]

rRtestit.log]]۶}_H_STH \
ז#n@ƽe;őq}C><<Ra?XbRi:OTL^r2(fMfދ \
w7r!z*k5O|iL2>xk|cw4{=/{]e4^PDH%zr0)^<$פ_J
 ^^ 0c}K^zI/c3l!?yW]l{_l{q;n1<j8mM \
GMCCM%ZMCU;d&WכVd5_|أ⿛m^svȧCO;?K9}O{{w{{~ԻΣ:jH`( \
o1,7yܨ4.)"75X#* xH)x3E:ΦQΏcYJ/"JaqTr \
'JMXBNJya闟~}[1_2jcNὬ{w;j  
:"A	.z@,4H@x=αP򥭰#=۽vm_
UmKy.oD]W./H܏l0^wc*"JazX!l@Cmdl(MX`Nh`u}6Y.mX.yWJ\ͥ \
#L{dIybcPm7A?W6goຯt=*YNjF(@ \
{[),j;:yD9.B/O"#:Ghd0&0`ji Y \
O俴U5$"HEsţRj3)L#dx/F8_daߨk2[S7Mt`l
 ;^o#PlyD@΁}<\l̾6oAS}#q 73 \
9|#}#h777"L/foTcd5E)Qυr:od|NPl<H \
dL&}PG)C~}j8 \
Ԥ29W+cj\{B]3:ðl}.r*:`Qّ@kê65eQr&Q$"UTUFO1vkH3
 (y$or%[M$gWN0M_Iz%kfY~RD?Z*L&\S-T6jtQ5!j5Rm94}ٖ:5/yQ`Pn[1Z \
Mjg^>S⩘.Y$^H$Y)H T5,di!%ZDsd2Ǧ"/d|釟Kd \
EU_$;L1 EF㻕LcqY	ߵd>kIg@06bXc՗6s1EFtL0A82%)'4Y4pP'yC \
jќ`>'	YZ6ƃ]0>l:B% uT]ۦ
MRL'GD{*LgC+d)ZOOްYYt
H}ON};dtd6:y|8Aq4$5X_X 2T3MNPUj"6yUFyUF*W!T
RA4'w7kbDr0Im=JDKj.% \
Ec{(]LH	Jyt0*\vJM򄒚UgI_5!Bf';cbb:l)lt2ιw'soE \
w{|+Cw{lӁݭЦS,XWc}R'"r!eO$|IxNf^;[/UM \
\إACױ=^e67,FyƭdGdnO99sbJ^hr?hЪ84[g]kGA6>T/}:]⬬wSy[]|OҦA \
-Lo߲8)/H,4\5\5\X5mWWͺ8t$PW_5Fe|ӂ2ߴ>7_TSïZΉmR$RS&l6dkMyJuʕeee$3
 M5rnVQwSW \
8t7U^YpII09|R=kFϦ\0RR{R/>xT],WBƫc5-2(fseB \
я]v:<ѹϑb1NbK0Xi	m]*B!4lW>l2*m9)(*\o9 \
; )+Y	L@>Kwx[E \
,Dz"ӖD+MAH,S|$(23(rH@sA/m˶=DBfuYe!=L\ߐicamʆ;@oC
 DF6esgitx?>2AՂqgcHGfFfdȒJ螎EYT+"v#<+3=vރ(y/ً=ٶvоgtբDNqS{&$PG<D\:s'):\>
 ,\qWvy*qwY̒6G%+TOx%2^1:N:t^MS:/c
 s8)n#'t貞ٗH98/'e \
˾}8/N2P|/͗|@}8/]ey|pʗ_N@}0el\/#~x:




[prev in list] [next in list] [prev in thread] [next in thread]

Configure | About | News | Add a list | Sponsored by KoreLogic