Re: pkg/57145: gmake: *** INTERNAL: readdir: Operation not supported. Stop.

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> Date: Tue, 20 Aug 2024 07:03:34 +0000
> From: mrg%NetBSD.org@localhost
> 
> this is a gmake bug.  it checks errno after readdir() but readdir() is
> not expected to set/clear errno for 'end of dir' condition, and thus
> converts some prior random error into a readdir error.

Not a gmake bug.  readdir is required to preserve errno in the
end-of-directory case:

DESCRIPTION

   [...]
   Applications wishing to check for error situations should set errno
   to 0 before calling readdir().  If errno is set to non-zero on
   return, an error occurred.

RETURN VALUE

   [...] When the end of the directory is encountered, a null pointer
   shall be returned and errno is not changed.

POSIX 2024: https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/functions/readdir.html
SUSv2 1997: https://pubs.opengroup.org/onlinepubs/7908799/xsh/readdir.html

It's been this way for decades and we just screwed it up in the event
that lseek fails.

The attached patch arranges to preserve errno in the end-of-directory
case (and in the success case), and to set errno in error cases.

However, I would like to add an automatic test for this.

> Date: Fri, 30 Aug 2024 21:52:34 +0000
> From: mrg%NetBSD.org@localhost
> 
> we store lseek() into dirp->dd_seek for later usage, and in the case
> that it returns -1 and sets errno (this PR shows EOPNOTSUPP but most
> instances i've and others have seen is EINVAL - mine happens on pure
> FFS obj, not in a chroot.)

Can you share your reproducer for this?  Maybe we can adapt it to an
atf test.

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1725116374 0
#      Sat Aug 31 14:59:34 2024 +0000
# Branch trunk
# Node ID f39f4bb8989435150b682bb087c507f41adcaf8d
# Parent  9aaaa3422f6c6a29e9679ebd97936a4038ca0060
# EXP-Topic riastradh-pr57145-readdirerrno
readdir(3): Preserve errno on end-of-directory.

PR pkg/57145: gmake: *** INTERNAL: readdir: Operation not supported.
Stop.

diff -r 9aaaa3422f6c -r f39f4bb89894 lib/libc/gen/readdir.c
--- a/lib/libc/gen/readdir.c	Thu Aug 29 13:39:42 2024 +0000
+++ b/lib/libc/gen/readdir.c	Sat Aug 31 14:59:34 2024 +0000
@@ -56,35 +56,50 @@ static char sccsid[] = "@(#)readdir.c	8.
 struct dirent *
 _readdir_unlocked(DIR *dirp, int skipdeleted)
 {
+	const int saved_errno = errno;
 	struct dirent *dp;
 
 	for (;;) {
 		if (dirp->dd_loc >= dirp->dd_size) {
 			if (dirp->dd_flags & __DTF_READALL)
-				return (NULL);
+				break;
 			dirp->dd_loc = 0;
 		}
 		if (dirp->dd_loc == 0 && !(dirp->dd_flags & __DTF_READALL)) {
 			dirp->dd_seek = lseek(dirp->dd_fd, (off_t)0, SEEK_CUR);
 			dirp->dd_size = getdents(dirp->dd_fd,
 			    dirp->dd_buf, (size_t)dirp->dd_len);
-			if (dirp->dd_size <= 0)
-				return (NULL);
+			if (dirp->dd_size == 0) /* end of directory */
+				break;
+			if (dirp->dd_size == -1) /* getdents sets errno */
+				return NULL;
+			if (dirp->dd_size < 0) { /* paranoia */
+				errno = EIO;
+				return NULL;
+			}
 		}
 		dp = (struct dirent *)
 		    (void *)(dirp->dd_buf + (size_t)dirp->dd_loc);
-		if ((intptr_t)dp & _DIRENT_ALIGN(dp))/* bogus pointer check */
-			return (NULL);
+		/* bogus pointer check */
+		if ((intptr_t)dp & _DIRENT_ALIGN(dp)) {
+			errno = EIO;
+			return NULL;
+		}
 		/* d_reclen is unsigned; no need to compare it <= 0 */
-		if (dp->d_reclen > dirp->dd_len + 1 - dirp->dd_loc)
-			return (NULL);
+		if (dp->d_reclen > dirp->dd_len + 1 - dirp->dd_loc) {
+			errno = EIO;
+			return NULL;
+		}
 		dirp->dd_loc += dp->d_reclen;
 		if (dp->d_ino == 0 && skipdeleted)
 			continue;
 		if (dp->d_type == DT_WHT && (dirp->dd_flags & DTF_HIDEW))
 			continue;
-		return (dp);
+		return dp;
 	}
+
+	errno = saved_errno;
+	return NULL;
 }
 
 struct dirent *