From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boaz Harrosh Subject: Re: [pnfs] [PATCH RFC v2 0/21] nfs4xdr cleanup v2 Date: Mon, 24 Aug 2009 15:50:47 +0300 Message-ID: <4A928CA7.8070306@panasas.com> References: <4A8571E2.8020800@panasas.com> <4A89338D.1040207@panasas.com> <1250513254.8475.20.camel@heimdal.trondhjem.org> <4A896125.2040506@panasas.com> <1250527692.20012.26.camel@heimdal.trondhjem.org> <4A9240C7.5090307@panasas.com> <1251115007.6325.9.camel@heimdal.trondhjem.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Benny Halevy , Chuck Lever , "J. Bruce Fields" , NFS list , pNFS Mailing List To: Trond Myklebust Return-path: Received: from ip67-152-220-67.z220-152-67.customer.algx.net ([67.152.220.67]:20686 "EHLO daytona.int.panasas.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752456AbZHXMux (ORCPT ); Mon, 24 Aug 2009 08:50:53 -0400 In-Reply-To: <1251115007.6325.9.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org> Sender: linux-nfs-owner@vger.kernel.org List-ID: On 08/24/2009 02:56 PM, Trond Myklebust wrote: > On Mon, 2009-08-24 at 10:27 +0300, Boaz Harrosh wrote: >> OK I used the new stuff by now, and I'm happy with everything >> but the above. I *absolutely* insist on this changing to: > > Feel quite free to insist, but the patch isn't going in. > I think your being irrational here. I'm not the first that wants this, it has been brought up again and again on the mailing list. The big majority of nfs/xdr programmers want this. In fact you are the *only one* who's against it. >> p = xdr_encode_word(p, foo); >> and >> p = xdr_decode_word(p, &foo); >> >> [xdr_{encode,decode}_word is defined differently but is only used in a couple >> of sunrpc files, the change of these places shall be added to this cleanup] >> >> I have checked this version of the definition: >> static inline __be32 * >> xdr_encode_word(__be32 *p, __u32 val) >> { >> *p++ = cpu_to_be32(val); >> return p; >> } >> >> static inline __be32 * >> xdr_decode_word(__be32 *p, __u32 *valp) >> { >> *valp = be32_to_cpu(*p++); >> return p; >> } >> >> under assembly with gcc -O2 and it gives the exact same result as the open code, >> so I do not see what can be said against it? > > It is unnecessary, Yes it is necessary, for the reader. (And the writer). it looks ugly, I think it is not, that's a matter of taste, no? The opposite is true, your option is the ugly one. > the latter form takes 2 pointers and > hides an assignment, it does an unconditional pointer increment. > So do all the other xdr_{de,en}code_xxx I don't see a choice. Some eggs most be broken when making a cake. It is totally uniform with the reset of the code which is my point. I don't want an alien looking code in the mids of very uniform bunch. And I don't want distracting information, I want the essence stated clearly. I don't write assembly and I don't right computer language, I write English (conforming to computer logic). > IOW: Just learn the meaning of 'pointer to __be32'. > What? I lost you, I don't understand what you mean? Boaz