[PATCH] test-lchown.h: "symlink ownership not supported" is indicated by EOPNOTSUPP

[PATCH] test-lchown.h: "symlink ownership not supported" is indicated by EOPNOTSUPP

[Ivan Zakharyaschev]

The "symlink ownership not supported" condition is expected to be
indicated by EOPNOTSUPP, as opposed to the "no support for ownership",
which is indicated by ENOSYS.

The latter condition is checked for earlier in this test on a
directory (rather than on a symlink). And if ENOSYS is raised for a
symlink, but not for a directory, this would be against the expected
behavior and should make the test fail.

This change helps this test to work correctly (i.e., to be skipped) if
gnulib's own implementation of lchown is used.

Such convention concerning the meaning of EOPNOTSUPP was also stated
in The Open Group Base Specifications Issue 6
IEEE Std 1003.1, 2004 Edition
(http://pubs.opengroup.org/onlinepubs/009604599/functions/lchown.html);
however, the EOPNOTSUPP case is removed in Issue 7, 2018 Edition
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/lchown.html).

gnulib's own implementation in lchmod.c follows this convention since
a62be9f4039b4499cfbb76e394cad2259d03fa84 (2004-08-07):

int
lchown (const char *file, uid_t uid, gid_t gid)
{
# if HAVE_CHOWN
#  if ! CHOWN_MODIFIES_SYMLINK
  struct stat stats;

  if (lstat (file, &stats) == 0 && S_ISLNK (stats.st_mode))
    {
      errno = EOPNOTSUPP;
      return -1;
    }
#  endif

  return chown (file, uid, gid);

# else /* !HAVE_CHOWN */
  errno = ENOSYS;
  return -1;
# endif
}
---
 tests/test-lchown.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-lchown.h b/tests/test-lchown.h
index bc10a5038..b1a7d4b12 100644
--- a/tests/test-lchown.h
+++ b/tests/test-lchown.h
@@ -124,7 +124,7 @@ test_lchown (int (*func) (char const *, uid_t, gid_t), bool print)
       return 77;
     }
   result = func (BASE "dir/link2", -1, -1);
-  if (result == -1 && errno == ENOSYS)
+  if (result == -1 && errno == EOPNOTSUPP)
     {
       ASSERT (unlink (BASE "dir/file") == 0);
       ASSERT (unlink (BASE "dir/link2") == 0);
-- 
2.17.1

Re: [PATCH] test-lchown.h: "symlink ownership not supported" is indicated by EOPNOTSUPP

[Bruno Haible]

Hi Ivan,

> Such convention concerning the meaning of EOPNOTSUPP was also stated
> in The Open Group Base Specifications Issue 6
> IEEE Std 1003.1, 2004 Edition
> (http://pubs.opengroup.org/onlinepubs/009604599/functions/lchown.html);
> however, the EOPNOTSUPP case is removed in Issue 7, 2018 Edition
> (http://pubs.opengroup.org/onlinepubs/9699919799/functions/lchown.html).

The fact that it has been removed from the later revision of the standard
means that implementations have now more freedom regarding the errno
value that they return in this case.

> And if ENOSYS is raised for a
> symlink, but not for a directory, this would be against the expected
> behavior and should make the test fail.

You claim "should", but I don't want to make the test fail when it earlier
succeeded; and you have not tested this on the various platforms. As
pointed out above, various errno values should be acceptable.

I've pushed this instead:


2018-12-19  Bruno Haible  <bruno@clisp.org>

	lchown tests: Be more permissive regarding errno values.
	Reported by Ivan Zakharyaschev <imz@altlinux.org>.
	* tests/test-lchown.h (test_lchown): Recognize EOPNOTSUPP as an
	alternative to ENOSYS.
	* modules/lchown-tests (Depends-on): Add 'errno'.
	* modules/fchownat-tests (Depends-on): Likewise.

diff --git a/modules/fchownat-tests b/modules/fchownat-tests
index 81adf7f..e5fb783 100644
--- a/modules/fchownat-tests
+++ b/modules/fchownat-tests
@@ -7,6 +7,7 @@ tests/signature.h
 tests/macros.h
 
 Depends-on:
+errno
 ignore-value
 intprops
 mgetgroups
diff --git a/modules/lchown-tests b/modules/lchown-tests
index c5bba89..42b9460 100644
--- a/modules/lchown-tests
+++ b/modules/lchown-tests
@@ -6,6 +6,7 @@ tests/signature.h
 tests/macros.h
 
 Depends-on:
+errno
 ignore-value
 intprops
 mgetgroups
diff --git a/tests/test-lchown.h b/tests/test-lchown.h
index bc10a50..8bf79b1 100644
--- a/tests/test-lchown.h
+++ b/tests/test-lchown.h
@@ -124,7 +124,7 @@ test_lchown (int (*func) (char const *, uid_t, gid_t), bool print)
       return 77;
     }
   result = func (BASE "dir/link2", -1, -1);
-  if (result == -1 && errno == ENOSYS)
+  if (result == -1 && (errno == ENOSYS || errno == EOPNOTSUPP))
     {
       ASSERT (unlink (BASE "dir/file") == 0);
       ASSERT (unlink (BASE "dir/link2") == 0);

Re: [PATCH] test-lchown.h: "symlink ownership not supported" is indicated by EOPNOTSUPP

[Ivan Zakharyaschev]

Hi Bruno,

On Thu, 20 Dec 2018, Bruno Haible wrote:

> > Such convention concerning the meaning of EOPNOTSUPP was also stated
> > in The Open Group Base Specifications Issue 6
> > IEEE Std 1003.1, 2004 Edition
> > (http://pubs.opengroup.org/onlinepubs/009604599/functions/lchown.html);
> > however, the EOPNOTSUPP case is removed in Issue 7, 2018 Edition
> > (http://pubs.opengroup.org/onlinepubs/9699919799/functions/lchown.html).
> 
> The fact that it has been removed from the later revision of the standard
> means that implementations have now more freedom regarding the errno
> value that they return in this case.

My guess actually was that the standard now requires that there is no such 
kind of failure at all, and this operation has always to be implemented in 
conforming OSes. But I don't want to make this claim.

> > And if ENOSYS is raised for a
> > symlink, but not for a directory, this would be against the expected
> > behavior and should make the test fail.
> 
> You claim "should", but I don't want to make the test fail when it earlier
> succeeded; and you have not tested this on the various platforms. As
> pointed out above, various errno values should be acceptable.
>
> I've pushed this instead:

I have no objections. Thank you!

> 2018-12-19  Bruno Haible  <bruno@clisp.org>
> 
> 	lchown tests: Be more permissive regarding errno values.
> 	Reported by Ivan Zakharyaschev <imz@altlinux.org>.
> 	* tests/test-lchown.h (test_lchown): Recognize EOPNOTSUPP as an
> 	alternative to ENOSYS.
> 	* modules/lchown-tests (Depends-on): Add 'errno'.
> 	* modules/fchownat-tests (Depends-on): Likewise.
> 
> diff --git a/modules/fchownat-tests b/modules/fchownat-tests
> index 81adf7f..e5fb783 100644
> --- a/modules/fchownat-tests
> +++ b/modules/fchownat-tests
> @@ -7,6 +7,7 @@ tests/signature.h
>  tests/macros.h
>  
>  Depends-on:
> +errno
>  ignore-value
>  intprops
>  mgetgroups
> diff --git a/modules/lchown-tests b/modules/lchown-tests
> index c5bba89..42b9460 100644
> --- a/modules/lchown-tests
> +++ b/modules/lchown-tests
> @@ -6,6 +6,7 @@ tests/signature.h
>  tests/macros.h
>  
>  Depends-on:
> +errno
>  ignore-value
>  intprops
>  mgetgroups
> diff --git a/tests/test-lchown.h b/tests/test-lchown.h
> index bc10a50..8bf79b1 100644
> --- a/tests/test-lchown.h
> +++ b/tests/test-lchown.h
> @@ -124,7 +124,7 @@ test_lchown (int (*func) (char const *, uid_t, gid_t), bool print)
>        return 77;
>      }
>    result = func (BASE "dir/link2", -1, -1);
> -  if (result == -1 && errno == ENOSYS)
> +  if (result == -1 && (errno == ENOSYS || errno == EOPNOTSUPP))
>      {
>        ASSERT (unlink (BASE "dir/file") == 0);
>        ASSERT (unlink (BASE "dir/link2") == 0);
>