Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 12 Apr 2009 15:23:38 +0200
From:      Christoph Mallon <christoph.mallon@gmx.de>
To:        Alexander Kabaev <kabaev@gmail.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Ed Schouten <ed@freebsd.org>
Subject:   Re: svn commit: r190919 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include
Message-ID:  <49E1EB5A.9010303@gmx.de>
In-Reply-To: <20090412090551.176f327c@kan.dnsalias.net>
References:  <200904111401.n3BE1108088009@svn.freebsd.org>	<20090411163528.GC46526@troutmask.apl.washington.edu>	<20090411124335.0600a72f@kan.dnsalias.net> <49E1E01C.90704@gmx.de> <20090412090551.176f327c@kan.dnsalias.net>

next in thread | previous in thread | raw e-mail | index | archive | help
Alexander Kabaev schrieb:
> On Sun, 12 Apr 2009 14:35:40 +0200
> Christoph Mallon <christoph.mallon@gmx.de> wrote:
> 
>> Alexander Kabaev schrieb:
>>> On Sat, 11 Apr 2009 09:35:28 -0700
>>> Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
>>>
>>>> On Sat, Apr 11, 2009 at 02:01:01PM +0000, Ed Schouten wrote:
>>>>> Author: ed
>>>>> Date: Sat Apr 11 14:01:01 2009
>>>>> New Revision: 190919
>>>>> URL: http://svn.freebsd.org/changeset/base/190919
>>>>>
>>>>> Log:
>>>>>   Simplify in/out functions (for i386 and AMD64).
>>>>>   
>>>>>   Remove a hack to generate more efficient code for port numbers
>>>>> below 0x100, which has been obsolete for at least ten years,
>>>>> because GCC has an asm constraint to specify that.
>>>>>   
>>>>>   Submitted by:	Christoph Mallon <christoph mallon gmx de>
>>>>>
>>>> I thought Christoph and bde were still hashing out the correctness
>>>> of this patch.
>>>>
>>>> http://lists.freebsd.org/pipermail/freebsd-amd64/2009-April/012064.html
>>>>
>>>> -- 
>>>> Steve
>>> The patch is inconsistent in regards to usage of volatile vs.
>>> __volatile even within itself. I think the code is sloppy and was
>>> not ready to be committed yet. Please fix or back out.
>> Backing it out because of two underscores (!) would be 
>> counterproductive: It removes about 150 lines of hard to read hacks, 
>> which are unnecessary for at least a decade. GCC 2.95, which was 
>> released in 1999 supports the "N" constraint for inline asm. Perhaps 
>> olders do, too, but you cannot get older GCCs from the official site.
>> Attached is a patch, which replaces all __inline and __volatile in
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> the touched headers by thir ISO equivalents - again there hasn't been
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> a reason to use the alternate GCC keywords for at least a decade.
>> Also "inline" and "volatile" are already used hundreds of times in
>> sys/. The patch is simply the result of
>> %s/\<__\(inline\|volatile\)\>/\1/.
>>
>> 	Christoph
> 
> Underscores as the sole reason for backing out might as well be
> counterproductive. The real objection was not about underscored
> though, but about consistency. Your patch was not consistent itself
> and it left files it touched with unholy mixture of plain and

We are all doomed! I can almost see the horsemen of apocalypse appearing 
at the edge of the horizon!
You're a /tad/ histrionic.

> underscored versions sprinkled all over with no apparent system.
> 
> The way to handle this change was:
> 1. Prepare functional patch, commit.
> 2. Prepare underscore removal patch, commit.

Did you stop reading at the colon of the first sentence? I marked the 
relevant part above with "^".

	Christoph



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49E1EB5A.9010303>