From owner-freebsd-hackers@freebsd.org Tue Feb 27 22:30:55 2018 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5FBFFF2FA86 for ; Tue, 27 Feb 2018 22:30:55 +0000 (UTC) (envelope-from torek@elf.torek.net) Received: from elf.torek.net (mail.torek.net [96.90.199.121]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "elf.torek.net", Issuer "elf.torek.net" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id D083674520 for ; Tue, 27 Feb 2018 22:30:54 +0000 (UTC) (envelope-from torek@elf.torek.net) Received: from elf.torek.net (localhost [127.0.0.1]) by elf.torek.net (8.15.2/8.15.2) with ESMTPS id w1RMUO2Q079463 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Tue, 27 Feb 2018 14:30:24 -0800 (PST) (envelope-from torek@elf.torek.net) Received: (from torek@localhost) by elf.torek.net (8.15.2/8.15.2/Submit) id w1RMUOmL079462 for freebsd-hackers@freebsd.org; Tue, 27 Feb 2018 14:30:24 -0800 (PST) (envelope-from torek) Date: Tue, 27 Feb 2018 14:30:24 -0800 (PST) From: Chris Torek Message-Id: <201802272230.w1RMUOmL079462@elf.torek.net> To: freebsd-hackers@freebsd.org Subject: Re: Marking select(2) as restrict In-Reply-To: <20180227210110.GA76452@stack.nl> X-Greylist: inspected by milter-greylist-4.6.2 (elf.torek.net [127.0.0.1]); Tue, 27 Feb 2018 14:30:24 -0800 (PST) for IP:'127.0.0.1' DOMAIN:'localhost' HELO:'elf.torek.net' FROM:'torek@elf.torek.net' RCPT:'' X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (elf.torek.net [127.0.0.1]); Tue, 27 Feb 2018 14:30:24 -0800 (PST) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Feb 2018 22:30:55 -0000 If we can back up a bit, there are (at least) two mostly-separate issues: * What types and qualfiiers to use for various parameters: These are dictated by standards, and we should just conform to whatever is in the standard unless there's some major reason to do otherwise. POSIX says that we *should* use "restrict" here: http://pubs.opengroup.org/onlinepubs/009696699/functions/pselect.html so if no one has a major reason to do otherwise (such as "no conforming program can tell that we didn't and all it will achieve is nothing" :-) ) I'd say add the restrict. (As kib and others have noted, it achieves a lot of nothing, so the priority for this particular conformance seems low.) * Select itself: it makes no sense *in the caller* to pass the same &fd_set argument to more than one parameter unless they are all input-only. The kernel is going to write on them, and there is no guarantee that the kernel will write them in any particular order. Hence if you write: ret = select(nfds, &a, &a, &a, timeout); you are nearly guaranteed to have a bug: this call can only be correct if the only value you intend to inspect is "ret". (That, in fact, is what devd was doing: if ret==0, daemonize, and in any case throw away the returned results in the by-pointer arguments. Hence not a bug -- unless ... well, read on.) The remaining question is: can even that be correct? In particular, should the kernel be required to use copy-in/copy-out semantics, or can it do the equivalent (minus error checking and a lot of other important details) of: int select(int nfds, fd_set *in, fd_set *out, fd_set *ex, struct timeval *tvp) { some_complicated_type v_in, v_out, v_ex; v1 = op(*in); *in = function(v1); v2 = op(*out); ... } ? If the latter is *permitted*, then the call passing "&a" three times is *always* wrong, since *in might have been clobbered before *out is ever loaded. If a program breaks because it was doing this, well, it was already broken, we were just lucky that it worked anyway. (Was this good luck, or bad luck? :-) ) Currently, the kernel does in fact copy them all in, then futz with them, then copy them all out (for good reasons). So the devd usage always works. But will the kernel *always* do this, or is that an accident of the implementation? If the kernel *is* always going to do this, the documentation should say so, at which point the POSIX requirement for the restrict on the parameter is itself a bug (as far as BSD is concerned). That would call for feature test macros around the declaration. So, if we declare that select(nfds, &a, &a, &a, timeout) *is* legal and meaningful in BSD, *don't* just put in the restrict: use feature test macros as appropriate to handle any conformance issues. But please also update select(2) to call out when this is OK (e.g., in devd's particular use case). Chris