Jose R. Santos wrote: > On Tue, 05 Jun 2007 15:26:53 +0200 Laurent Vivier <[EMAIL PROTECTED]> > wrote: > >> Dave Kleikamp wrote: >>> Jose is right. The endian conversion is unnecessary. >>> >>> Shaggy >> But by using le32_to_cpu(es->s_blocks_count_hi) you explicitly mark the >> variable as a little-endian. So if someone reads the code, he knows this is >> a little-endian value and this allows to avoid errors if later variable >> must be tested for other value than 0. >> >> For instance, you have : >> >> if(es->s_blocks_count_hi) >> >> and later the value should be compared to 10, how do you know easily you >> should use: >> >> if (le32_to_cpu(es->s_blocks_count_hi) == 10) >> >> instead of >> >> if(es->s_blocks_count_hi == 10) >> >> I think writing like Mingming asks should allow to avoid errors later. >> >> (and code becomes really self-explicit...) >> >> Regards, Laurent > > Hi Laurent, > > In this particular case though, the value of s_blocks_count_hi should not be > uses on its own. The correct way would be to use ext4_blocks_count() which > already does the endian conversion. If you think the code could confuse > people as to how to access the data in s_blocks_count_hi, wouldn't hiding it > through the use of a macro make more sense than doing an unnecessary endian > conversion? >
Yes, I think the code could confuse people, but I don't think defining "Yet
Another Macro" is a good choice (IMHO).
I think we can resolve this (non-)issue by two ways:
- using le32_to_cpu() (but I agree it does an unnecessary endian conversion on
big-endian systems)
- put a comment on the line (but are we allowed to put comments in kernel source
code... ;-) )
Regards
Laurent
--
------------- [EMAIL PROTECTED] --------------
"Any sufficiently advanced technology is
indistinguishable from magic." - Arthur C. Clarke
signature.asc
Description: OpenPGP digital signature
