bin/102510: diff should not follow symlink in recursive (-r) mode

[Jin Guojun]

>Number:         102510
>Category:       bin
>Synopsis:       diff should not follow symlink in recursive (-r) mode
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Aug 25 18:50:18 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Jin Guojun
>Release:        All FreeBSD release
>Organization:
>Environment:
Non hardware related issue
>Description:
/usr/bin/diff should not follow symlink in recursive mode (with option "-r").

Where "-r" option is used to compare two structures/trees, where one of them is a
mirror or duplication of the other. 
It is not clear what is the historical reason "diff -r" follows symlink in
this mode. It is hardly to see "diff -r" may be used to compare two completely
different file structure or trees.

Therefore, to compare two same/similar file trees, there is no any good reason
to follow the symlink for diffing. 
If the node at the end of the link exists, it will be compared eventually anyway.
If the symlink points to nowhere, then there is no meaning to follow it.

It wastes time to follow symlink to do number of extra comparisons.
Also, if user(s) create some long-multi-path links between directories/trees,
diff -r will loop forever (no infinit, but may take a day) in comparing such
file trees.


>How-To-Repeat:
I have send a simple example to hackers for comment, but have not heard any feedback. It can be found in mailing archive.

The real problem on a large file tree/structure may take time to debug.
However, it is not hard to see the problem through above description.

A patch is provide to disable the feature of following symlink in "-r" mode.
Since the patch is copy/pasted, it may not be directly applied via "patch",
but it is very simple (tree line added, and one line changed), so manually
apply it is easy. When I receive the reply,I will send the patch in via email
if people agree this patch.

If for some reason following symlink is needed in "-r" mode, then a switch
(option) can be added for such purpose to enable following symlink feature
in "-r" recursive mode.
>Fix:
*** /usr/src/contrib/diff/diff.c.orig   Tue Aug 15 14:05:30 2006
--- /usr/src/contrib/diff/diff.c        Tue Aug 15 14:04:45 2006
***************
*** 832,837 ****
--- 832,840 ----

    /* other popular file types */
    /* S_ISLNK is impossible with `fstat' and `stat'.  */
+ #ifdef S_ISLNK
+   if (S_ISLNK (st->st_mode)) return "symlink";
+ #endif
  #ifdef S_ISSOCK
    if (S_ISSOCK (st->st_mode)) return "socket";
  #endif
***************
*** 927,933 ****
                }
            }
          else
!           stat_result = stat (inf[i].name, &inf[i].stat);

          if (stat_result != 0)
            {
--- 930,936 ----
                }
            }
          else
!           stat_result = (recursive ? lstat : stat) (inf[i].name, &inf[i].stat)
;

          if (stat_result != 0)
            {

>Release-Note:
>Audit-Trail:
>Unformatted: