Re: [PATCH 2/2] New internal function __access_noerrno

[Kalle Olavi Niemitalo <kon@iki.fi>]

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> +/* __access variant that does not set errno.  Used in very early initialization
> +   code in libc.a and ld.so.  */
> +extern __typeof (__access) __access_noerrno attribute_hidden;

Please document what __access_noerrno is supposed to return on error.
The NaCl and Linux implementations appear to return the error code,
but the stub and Hurd implementations return -1 like access does,
so I'm not sure what you intended.
In "[PATCH 1/4] Add framework for tunables", the caller
only checks whether the result is zero or nonzero.

> +__hurd_fail_noerrno (error_t err)

Your __hurd_fail_noerrno just returns (err == 0 ? 0 : -1).
All the assignments to err in the switch statement are dead.

If __access_noerrno is not required to return the error code,
then I don't think __hurd_fail_noerrno is necessary at all...

> +static int
> +hurd_fail_noerrno (error_t err)
> +{
> +  return __hurd_fail_noerrno (err);
> +}

... instead, hurd_fail_noerrno could just return -1
unconditionally, because it is not even called if err == 0.
(Perhaps then rename it, to avoid suggesting it's similar to __hurd_fail.)

> -    return __hurd_fail (EACCES);
> +    return errfunc (EACESS);

Typo here.

The t/faccessat branch at git://git.sv.gnu.org/hurd/glibc.git
changes this code path quite a lot.  Until that branch is merged
to upstream glibc, I think your function-pointer scheme is OK,
apart from the comments above.
I didn't yet try building it though.