Trond Myklebust wrote: > On Mon, 2009-08-17 at 13:40 +0300, Boaz Harrosh wrote: >> * So I hate it that the READ32/WRITE32 is open coded. Now it looks like nothing, it >> should have an xdr_encode/decode naming convention, all details hidden, consistent calling >> convention with the reset of the code. > > The convention is already established. Please see 15 years worth of > NFSv2/v3 xdr code. The NFSv4 code was the exception not the rule. > > Now, it is obvious what we're doing: we're writing 32-bit big endian > encoded data into a memory buffer. Furthermore, the process of > converting from cpu ordered integers into 32-bit big endian is now made > obvious, making it easily verifiable both by inspection, and using > 'sparse'. > Adding wrappers to do this, is just unnecessarily obfuscating the code. > >> * I hate it that I have to count (p)"++" now to look for actual code advancements. > > How is that _any_ different from counting WRITE32s? > >> * I hate that it is called encode_hyper that a dum like me needs to look it up in the dictionary >> "32" and "64" will do > > I don't mind changing the name of xdr_encode_hyper() to > xdr_encode_uint64 or some such alternative, however the point is that we > should have only _one_ function that encodes 64 bit integers. Not one > function that everyone else uses, and then a macro that NFSv4 uses. > It seems to me that xdr_encode_hyper() is good because it matches the XDR specification, which called 64 bit integer values, "hyper". ps >> * I hated it before, but I hate it even more now that the same operation is: >> reserved(4); p + 1 > > Huh? What operation? > >> I wish it could be all __be32 everywhere > > ??? That's up to the protocol. If the protocol uses 32-bit integers, > then we can use __be32, but if it specifies a 64-bit big endian integer > in some structure, then we need to encode for that. > -- 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
