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
--
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
