On Mon, 2009-08-24 at 15:50 +0300, Boaz Harrosh wrote:
> 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.
I saw your call for a show of hands, but have yet to see any replies. It
is probably a bit early for you to start talking about big majorities...
In any case, I don't apply patches based on popular vote. I apply them
based on my conviction that they are useful.
> >> 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?
I mean that the fundamental type of XDR is a __be32. We are using that
fundamental type in standard C code.
The fundamental Linux function for converting from a u32 to a __be32 is
cpu_to_be32(). It doesn't need a new wrapper, and neither does
be32_to_cpu().
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
