On Sun, Sep 26, 2021 at 08:10:53AM -0600, Todd C. Miller wrote:
> On Sun, 26 Sep 2021 15:48:19 +0200, Christian Weisgerber wrote:
> 
> > This is in itself a portability problem.  I don't think Linux even
> > has EFTYPE.
> >
> > We'll need to do something like this in -portable:
> >
> >         return (errno == ELOOP
> > #if   defined(__FreeBSD__)
> >             || errno == EMLINK
> > #elif defined(__NetBSD__)
> >             || errno == EFTYPE
> > #endif
> >             );
> 
> Everything is terrible.  I still think we are better off with #ifdefs
> testing for the existence of the errno define and not a specific OS.
> 
>  - todd
 
How about this?


diff refs/heads/main refs/heads/open-nofollow
blob - 66b864fee620072787f5c6149732f3c409f4f0cf
blob + 0bf2f902eb9f9a5b3567b36615d2d0c01f8f1b80
--- got/got.c
+++ got/got.c
@@ -4394,7 +4394,7 @@ print_diff(void *arg, unsigned char status, unsigned c
 		if (dirfd != -1) {
 			fd = openat(dirfd, de_name, O_RDONLY | O_NOFOLLOW);
 			if (fd == -1) {
-				if (errno != ELOOP) {
+				if (!got_err_open_nofollow_on_symlink()) { 
 					err = got_error_from_errno2("openat",
 					    abspath);
 					goto done;
@@ -4407,7 +4407,7 @@ print_diff(void *arg, unsigned char status, unsigned c
 		} else {
 			fd = open(abspath, O_RDONLY | O_NOFOLLOW);
 			if (fd == -1) {
-				if (errno != ELOOP) {
+				if (!got_err_open_nofollow_on_symlink()) {
 					err = got_error_from_errno2("open",
 					    abspath);
 					goto done;
blob - f87c06cad9b95c07cc3e6ce8f0e0f733de70a1a6
blob + e1becc061137b20d5e2b85ad0fb8830565d9e2db
--- include/got_error.h
+++ include/got_error.h
@@ -427,3 +427,9 @@ const struct got_error *got_error_path(const char *, i
  * additional arguments.
 */
 const struct got_error *got_error_fmt(int, const char *, ...);
+
+/*
+ * Check whether open(2) with O_NOFOLLOW failed on a symlink.
+ * This must be called directly after open(2) because it uses errno!
+ */
+int got_err_open_nofollow_on_symlink(void);
blob - 72e00488bedc8ea49b3e20af7eb76ec82839bfa3
blob + a501eee772d846c2fa0e88862b9d7422c8380be9
--- lib/error.c
+++ lib/error.c
@@ -258,3 +258,26 @@ got_error_fmt(int code, const char *fmt, ...)
 
 	abort();
 }
+
+int
+got_err_open_nofollow_on_symlink(void)
+{
+	if (errno == 0)
+		return 0;
+	/*
+	 * Check whether open(2) with O_NOFOLLOW failed on a symlink.
+	 * Posix mandates ELOOP and OpenBSD follows it. Others return
+	 * different error codes. We carry this workaround to help the
+	 * portable version a little.
+	 */
+#ifndef ELOOP
+#define ELOOP 0
+#endif
+#ifndef EMLINK
+#define EMLINK 0
+#endif
+#ifndef EFTYPE
+#define EFTYPE 0
+#endif
+	return (errno == ELOOP || errno == EMLINK || errno == EFTYPE);
+}
blob - d2c6246c748ba52478619a448ec02bf61c9e9729
blob + 1092bb03e5e276167684b61088c369b208953167
--- lib/object_create.c
+++ lib/object_create.c
@@ -129,7 +129,7 @@ got_object_blob_file_create(struct got_object_id **id,
 
 	fd = open(ondisk_path, O_RDONLY | O_NOFOLLOW);
 	if (fd == -1) {
-		if (errno != ELOOP)
+		if (!got_err_open_nofollow_on_symlink())
 			return got_error_from_errno2("open", ondisk_path);
 
 		if (lstat(ondisk_path, &sb) == -1) {
blob - 904fb23ceb80ef07dc958d9908801822c83de3f3
blob + 0c6b89ce6d761d53511099314379119479ee725e
--- lib/worktree.c
+++ lib/worktree.c
@@ -1284,7 +1284,7 @@ replace_existing_symlink(int *did_something, const cha
 	 */
 	fd = open(ondisk_path, O_RDWR | O_EXCL | O_NOFOLLOW);
 	if (fd == -1) {
-		if (errno != ELOOP)
+		if (!got_err_open_nofollow_on_symlink())
 			return got_error_from_errno2("open", ondisk_path);
 
 		/* We are updating an existing on-disk symlink. */
@@ -1781,9 +1781,10 @@ get_file_status(unsigned char *status, struct stat *sb
 		}
 	} else {
 		fd = open(abspath, O_RDONLY | O_NOFOLLOW);
-		if (fd == -1 && errno != ENOENT && errno != ELOOP)
+		if (fd == -1 && errno != ENOENT &&
+		    !got_err_open_nofollow_on_symlink())
 			return got_error_from_errno2("open", abspath);
-		else if (fd == -1 && errno == ELOOP) {
+		else if (fd == -1 && got_err_open_nofollow_on_symlink()) {
 			if (lstat(abspath, sb) == -1)
 				return got_error_from_errno2("lstat", abspath);
 		} else if (fd == -1 || fstat(fd, sb) == -1) {
@@ -3767,7 +3768,7 @@ worktree_status(struct got_worktree *worktree, const c
 	fd = open(ondisk_path, O_RDONLY | O_NOFOLLOW | O_DIRECTORY);
 	if (fd == -1) {
 		if (errno != ENOTDIR && errno != ENOENT && errno != EACCES &&
-		    errno != ELOOP)
+		    !got_err_open_nofollow_on_symlink())
 			err = got_error_from_errno2("open", ondisk_path);
 		else {
 			if (!no_ignores) {
@@ -4473,7 +4474,7 @@ create_patched_content(char **path_outfile, int revers
 	if (dirfd2 != -1) {
 		fd2 = openat(dirfd2, de_name2, O_RDONLY | O_NOFOLLOW);
 		if (fd2 == -1) {
-			if (errno != ELOOP) {
+			if (!got_err_open_nofollow_on_symlink()) {
 				err = got_error_from_errno2("openat", path2);
 				goto done;
 			}
@@ -4487,7 +4488,7 @@ create_patched_content(char **path_outfile, int revers
 	} else {
 		fd2 = open(path2, O_RDONLY | O_NOFOLLOW);
 		if (fd2 == -1) {
-			if (errno != ELOOP) {
+			if (!got_err_open_nofollow_on_symlink()) {
 				err = got_error_from_errno2("open", path2);
 				goto done;
 			}