Re: [PATCH 3/5] Linux: Emulate fchmodat with AT_SYMLINK_NOFOLLOW using O_PATH [BZ #14578]

[Paul Eggert <eggert at cs dot ucla dot edu>]

On 1/22/20 12:03 PM, Florian Weimer wrote:
> +	  if (errno == ENFILE || errno == EMFILE)
> +	    /* These errors cannot happen with a straight fchmodat
> +	       operation because it does not create file descriptors,
> +	       so hide them.  */
> +	    __set_errno (EOPNOTSUPP);
> +	  /* Otherwise, this should accurately reflect the expected
> +	     error from fchmodat (e.g., EBADF or ENOENT).  */

These lines can be omitted. I omitted them when recently adding similar code to 
Gnulib in <https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/fchmodat.c>. 
There's no need for fchmodat to masquerade ENFILE and EMFILE as EOPNOTSUPP: 
POSIX does not require it and masquerading those two errno values would be 
misleading, in the sense that it would make it harder for the caller to diagnose 
what actually went wrong.

> +      char buf[32];
> +      if (__snprintf (buf, sizeof (buf), "/proc/self/fd/%d", pathfd) < 0)
> +	{
> +	  __close_nocancel (pathfd);
> +	  return INLINE_SYSCALL_ERROR_RETURN_VALUE (EOPNOTSUPP);
> +	}

Similarly here. Also, there should be no reason for snprintf to fail. One 
possibility is to replace this with what's in Gnulib:

  static char const fmt[] = "/proc/self/fd/%d";
  char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
  sprintf (buf, fmt, pathfd);

where INT_BUFSIZE_BOUND is taken from <intprops.h>.

Otherwise, this patch series looks OK to me; thanks for writing it.