Jose R. Santos wrote: > On Tue, 05 Jun 2007 15:26:53 +0200 Laurent Vivier <Laurent.Vivier@xxxxxxxx> > 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 -- ------------- Laurent.Vivier@xxxxxxxx -------------- "Any sufficiently advanced technology is indistinguishable from magic." - Arthur C. Clarke
Attachment:
signature.asc
Description: OpenPGP digital signature