From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 97062 invoked by alias); 17 Nov 2016 20:53:21 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 97035 invoked by uid 89); 17 Nov 2016 20:53:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,SPF_NEUTRAL autolearn=ham version=3.3.2 spammy=H*F:D*fi, Hx-languages-length:1546 X-HELO: emh06.mail.saunalahti.fi From: Kalle Olavi Niemitalo To: libc-alpha Subject: Re: [PATCH 2/2] New internal function __access_noerrno In-Reply-To: (Adhemerval Zanella's message of "Wed, 16 Nov 2016 11:44:35 -0200") References: <1478797446-12213-1-git-send-email-adhemerval.zanella@linaro.org> <1478797446-12213-2-git-send-email-adhemerval.zanella@linaro.org> <083c6da8-5c67-acba-f7f0-79fb4253d7d2@gotplt.org> <23ef1354-7ade-70fe-46f0-59392798ef82@linaro.org> <60199742-d6f4-3f2a-ad38-619dcfdc7954@linaro.org> User-Agent: Gnus/5.110007 (No Gnus v0.7) Emacs/23.0.51 (gnu/linux) Keywords: Hurd,review Date: Thu, 17 Nov 2016 20:53:00 -0000 Message-ID: <87r369j093.fsf@Niukka.kon.iki.fi> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2016-11/txt/msg00641.txt.bz2 Adhemerval Zanella 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.