On 08/17/2009 07:48 PM, Trond Myklebust wrote:
> On Mon, 2009-08-17 at 16:54 +0300, Boaz Harrosh wrote:
>> On 08/17/2009 03:47 PM, Trond Myklebust wrote:
>>> Adding wrappers to do this, is just unnecessarily obfuscating the code.
>>>
>>
>> As I said, *sometimes* "obfuscation" is the point. If it has merits to account
>> for human short concentration span.
>
> In this case, the change was motivated by a bunch of patches which were
> doing things like adding dummy READ32 requests immediately prior to a
> READ_BUF() call. It was therefore abundantly clear that the obfuscation
> was helping no-one, even though it may have made it easy to churn out
> bad code quickly.
>
>>>> * I hate it that I have to count (p)"++" now to look for actual code advancements.
>>>
>>> How is that _any_ different from counting WRITE32s?
>>>
>>
>> The size for one. And mainly the symmetry with the other side of the wire.
>>
>> WRITEXX for every READXX. What now?
>
> A
>
> *p++ = cpu_to_be32(foo); // WRITE32(foo)
>
> for every
>
> foo = be32_to_cpu(*p++); // READ32(foo)
>
OK I used the new stuff by now, and I'm happy with everything
but the above. I *absolutely* insist on this changing to:
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?
All in favor say I. (And even if you don't, I know what you all think ;-))
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
