Subject: bug#71486: closed (Re: [bug-diffutils] bug#71486: no-dereference test fails on FreeBSD, NetBSD)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 12 Jun 2024 00:52:03 +0000

Your bug report

#71486: no-dereference test fails on FreeBSD, NetBSD

which was filed against the diffutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
71486: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71486
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [bug-diffutils] bug#71486: no-dereference test fails on FreeBSD, NetBSD
From: Paul Eggert <[email-masked]>
To: Bruno Haible <[email-masked]>
Cc: [email-masked]
Date: Tue, 11 Jun 2024 17:51:41 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for reporting that. I installed the attached patch which I hopes 
fixes it, and am boldly closing the bug report.

From d636f9c117d6383fedcc769661fa0fa4d372b153 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Tue, 11 Jun 2024 17:48:14 -0700
Subject: [PATCH] diff: port to FreeBSD, NetBSD

Problem reported by Bruno Haible <https://bugs.gnu.org/71486>.
* src/diff.c (NOFOLLOW_SYMLINK_ERRNO): New constant.
(compare_files): Use it instead of ELOOP.  When it is not ELOOP,
treat ELOOP as a failure in resolving the parent directory;
this saves a syscall in some situation.
---
 src/diff.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/diff.c b/src/diff.c
index 76432ee..3abcb66 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -1139,6 +1139,20 @@ dir_p (struct comparison const *pcmp, int f)
   return S_ISDIR (pcmp->file[f].stat.st_mode) != 0;
 }
 
+/* If openat with O_NOFOLLOW fails because the file is a symlink,
+   this platform sets errno to NOFOLLOW_SYMLINK_ERRNO.
+   Although POSIX says errno must be ELOOP in that situation,
+   FreeBSD and NetBSD behave more usefully.  */
+enum { NOFOLLOW_SYMLINK_ERRNO =
+#ifdef __FreeBSD__
+	 EMLINK
+#elif defined __NetBSD__
+	 EFTYPE
+#else
+	 ELOOP
+#endif
+};
+
 /* Compare two files with parent comparison PARENT.
    The two files are described by CMP, which has been prepped to contain
    the files' stat results, file types, and possibly descriptors.
@@ -1485,10 +1499,14 @@ compare_files (struct comparison const *parent, enum detype const detype[2],
 		    err = 0;
 		}
 
-	      /* If it might be a symlink, play it safe and fstatat later.  */
-	      if (err == ELOOP && no_dereference_symlinks
-		  && (detype[f] == DE_UNKNOWN
-		      || (detype[f] == DE_LNK && accmode == O_RDONLY)))
+	      /* If it is a symlink, fstatat later.  If it might be a
+		 symlink, play it safe and fstatat later.  */
+	      if (err == NOFOLLOW_SYMLINK_ERRNO
+		  && (NOFOLLOW_SYMLINK_ERRNO != ELOOP
+		      || (no_dereference_symlinks
+			  && (detype[f] == DE_UNKNOWN
+			      || (detype[f] == DE_LNK
+				  && accmode == O_RDONLY)))))
 		{
 		  fd = UNOPENED;
 		  err = 0;
@@ -1561,11 +1579,12 @@ compare_files (struct comparison const *parent, enum detype const detype[2],
 	       : openat (dirfd, atname, O_RDONLY | oflags));
 	  if (O_PATH_DEFINED && cmp.file[dir_arg].desc < 0
 	      && (dir_detype == DE_LNK || dir_detype == DE_UNKNOWN)
-	      && no_dereference_symlinks && errno == ELOOP)
+	      && no_dereference_symlinks && errno == NOFOLLOW_SYMLINK_ERRNO)
 	    cmp.file[dir_arg].desc = openat (dirfd, atname,
 					     O_PATHSEARCH | oflags);
 	  if (cmp.file[dir_arg].desc < 0
-	      ? (O_PATH_DEFINED || !no_dereference_symlinks || errno != ELOOP
+	      ? (O_PATH_DEFINED || !no_dereference_symlinks
+		 || errno != NOFOLLOW_SYMLINK_ERRNO
 		 || (fstatat (dirfd, atname, &cmp.file[dir_arg].stat,
 			      AT_SYMLINK_NOFOLLOW)
 		     < 0))
-- 
2.45.2

Subject: no-dereference test fails on FreeBSD, NetBSD
From: Bruno Haible <[email-masked]>
To: [email-masked]
Date: Tue, 11 Jun 2024 22:20:42 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

On FreeBSD 14.0 and NetBSD 10.0, the unit test 'no-dereference' fails.

How to reproduce:

Use current diffutils git master.

$ ln -s loop loop
$ mkdir subdir
$ ln -s loop subdir/loop

# This part succeeds on all platforms:
$ src/diff --no-dereference loop subdir/loop
$ echo $?
0

# This commands succeeds on all platforms except FreeBSD and NetBSD
# but fails on FreeBSD and NetBSD.
$ src/diff --no-dereference loop subdir
diff: subdir/loop: Too many links

The manual [1] says:
  "If one file is a directory and the other is not, diff compares the file
   in the directory whose name is that of the non-directory."
But single-stepping through the code, I don't get through code that
references 'subdir/loop'. It merely references 'subdir'.

The stack trace is:
  diff.c:880
    -> diff.c:1608
         -> error

Bruno

[1] https://www.gnu.org/software/diffutils/manual/html_node/Invoking-diff.html