From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/9478 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] slim down and avoid undefined behavior in unsetenv Date: Sat, 5 Mar 2016 11:30:34 -0500 Message-ID: <20160305163034.GI9349@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1457195454 27827 80.91.229.3 (5 Mar 2016 16:30:54 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 5 Mar 2016 16:30:54 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-9491-gllmg-musl=m.gmane.org@lists.openwall.com Sat Mar 05 17:30:53 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1acF6S-00049y-0Y for gllmg-musl@m.gmane.org; Sat, 05 Mar 2016 17:30:52 +0100 Original-Received: (qmail 28322 invoked by uid 550); 5 Mar 2016 16:30:49 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 28296 invoked from network); 5 Mar 2016 16:30:48 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:9478 Archived-At: On Sun, Feb 21, 2016 at 12:51:30PM +0300, Alexander Monakov wrote: > > errno = EINVAL; > > return -1; > > } > > Some places in musl tail-call to __syscall_ret(-Exxx) to set errno. I wonder > if it's accidental or there's a guideline for using one style or the other? A large portion of the source files in musl are written purely with public APIs. This is especially desirable for legacy functions, where there's no reason to optimize them and where being maintenance-free is the most valuable property, but it's also nice for other files from the standpoints of maintenance, ease of reading, and ease of reuse in other software (as drop-ins for systems lacking working versions of these functions) or libcs. With this in mind, __syscall_ret is mainly used in places where the surrounding code already deals directly with making syscalls or does other things that necessitate use of libc-internal APIs. This might not be followed everywhere as a strict rule, but it's the intent. > The only place I imagine the tailcall style might be undesired is sem_trywait, > where returning failure is not expected to be rare. > > What do you think about a change that introduces __set_errno that accepts > positive errno and returns -1L? With that change __syscall_ret can become > > return r < -4095UL ? r : __set_errno(-r); I don't see much value to it; return __set_errno(e) is functionally equivalent to return __syscall_ret(-e) and just saves one "neg" instruction before the tailcall. It might be a nicer abstraction in places where we don't use syscalls directly, but for the most part musl avoids internal APIs in those situations anyway, as explained above. > > -again: > [snip] > > + for (char **e = __environ; *e; ) > > + if (!memcmp(name, *e, l) && l[*e] == '=') { > > Here the usage of memcmp requires that it scans buffers left-to-right and > stops on first mismatch. As I understand the standards do not guarantee that, > but musl's current implementation does, and is not interposable. Still, a > gotcha. I think you're right. Would it be better to switch such usage to strncmp then? Rich