From owner-svn-src-all@FreeBSD.ORG Wed Feb 27 04:46:09 2013 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 6669995; Wed, 27 Feb 2013 04:46:09 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id E282D912; Wed, 27 Feb 2013 04:46:08 +0000 (UTC) Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r1R4jv4Y018237 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 27 Feb 2013 15:45:59 +1100 Date: Wed, 27 Feb 2013 15:45:57 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Davide Italiano Subject: Re: svn commit: r247300 - in head: sys/sys usr.bin/truss In-Reply-To: Message-ID: <20130227144854.F1004@besplex.bde.org> References: <201302260213.r1Q2D2N1016801@svn.freebsd.org> <201302260941.52534.jhb@freebsd.org> <201302261335.38001.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=D4sfsYtj c=1 sm=1 a=BX3CFZHxMzQA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=Zxvf0eAqCQgA:10 a=6I5d2MoRAAAA:8 a=TSbVqHtbAAAA:8 a=0f5610cAo4tEP4S7ggQA:9 a=CjuIK1q_8ugA:10 a=SV7veod9ZcQA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Xin LI , John Baldwin X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Feb 2013 04:46:09 -0000 On Tue, 26 Feb 2013, Davide Italiano wrote: > On Tue, Feb 26, 2013 at 7:35 PM, John Baldwin wrote: >> On Tuesday, February 26, 2013 12:09:42 pm Davide Italiano wrote: >>> On Tue, Feb 26, 2013 at 3:41 PM, John Baldwin wrote: >>>> On Monday, February 25, 2013 9:13:02 pm Xin LI wrote: >>>>> Author: delphij >>>>> Date: Tue Feb 26 02:13:02 2013 >>>>> New Revision: 247300 >>>>> URL: http://svnweb.freebsd.org/changeset/base/247300 >>>>> >>>>> Log: >>>>> Expose timespec and timeval macros when __BSD_VISIBLE is defined. This >>>>> allows userland application to use the following macros: >>>>> >>>>> timespecclear, timespecisset, timespeccmp, timespecadd, >>>>> timespecsub; >>>>> >>>>> timevalclear, timevalisset, timevalcmp. These were intentionally left out. Unfortunately the API/namespace police were not vigilant when the even worse APIs timerclear(), etc., were imported from NetBSD/OpenBSD. Apart from namespace pollution, bugs in these APIs start with them being unsafe macros spelled as safe ones. This bug is unimportant for relatively limited use in the kernel, but is unsuitable for a public API. Not leaving the above out gives a reasonable but nonstandard set of APIs for timespecs (the 5 listed above), but a weird sesqui-duplicated set for timevals (the 3 listed above, plus the 5 misnamed timer*() from NetBSD/ OpenBSD). Not leaving out the 3 duplicates the 3 least useful ones. >>>> Why not fix truss to use the stock functions instead of keeping private >>>> "unusual" versions? They are 3-operand variants of the 2-operand kernel ones that escaped. 3 operands would be better (more general, and at no cost except in calling complexity if the target operand is one of the source operands), but the order of the 3 operands is as badly designed as possible: kernel API: operands (v, u); result v op= u; truss API: operands (t, u, v); result should be t = u op v result is v = t op u (just reversed; t is not the target...) The timeval APIs are not actually sesqui-duplicated like I said above. The NetBSD/OpenBSD ones follow the truss order, with t not being the target (it just means generic timespec/timeval, while u and v mean the second and third ones, and now it is the kernel API that is confusing since there is no t and the order of the others is reversed in the operands), while the 3 recently exposed kernel ones aren't of the form target = source1 op source2. truss's timespecadd() also has or had an off by 1 error in its tv_nsec wrap check. >>> time.h is already a mess in terms of namespace pollution, and this >>> exposure might not help thing. >>> Other details here: >>> http://permalink.gmane.org/gmane.os.freebsd.architechture/15518 The bintime macros add further designed and implementation errors. And now there are sbintimes... >> I think that is orthogonal. Even if this is reverted I think truss should >> be changed to use the "normal" timespecsubt() macro rather than using a custom >> one with a different argument order. > > When I talked about "exposure" I referred about timeval/timespec > macros(). I wasn't arguing about your proposed change. > Sorry if it wasn't clear. In fact, conversion would further unimprove truss, since it wants a different target in all cases, so the 3-operand variants are ideal for it. Converting to the 2-operand variants would require a temporary variable in all callers or in the macros. The macros could be based on the standard ones, but would hardly be improved by that since the operations are so simple that no one would get them wrong with off by 1 errors etc. Bruce