[Ocfs2-tools-devel] [PATCH 9/9] dx_dirs: fix ocfs2_swap_dx_entry_list() for big endian

[Tao Ma]

Coly Li wrote:
> 
> On 04/27/2010 09:40 AM, Tao Ma Wrote:
>> Hi coly,
>>
>> Coly Li wrote:
> [snip]
>>> +/* called for big endian */
>>> +static void ocfs2_swap_dx_entry_list(struct ocfs2_dx_entry_list
>>> *dl_list, int to_cpu)
>>>  {
>>>      int i;
>>>
>>> -    dl_list->de_count    = bswap_16(dl_list->de_count);
>>> -    dl_list->de_num_used    = bswap_16(dl_list->de_num_used);
>>> +    if (to_cpu)
>>> +        dl_list->de_count = bswap_16(dl_list->de_count);
>>>
>>>      for (i = 0; i < dl_list->de_count; i++)
>>>          ocfs2_swap_dx_entry(&dl_list->de_entries[i]);
>>> +    dl_list->de_num_used = bswap_16(dl_list->de_num_used);
>>> +
>>> +    if (!to_cpu)
>>> +        dl_list->de_count = bswap_16(dl_list->de_count);
>>>  }
>> why change like this? You already have ocfs2_swap_dx_entry_list_to_cpu
>> and ocfs2_swap_dx_entry_list_from_cpu. So why not change these 2
>> functions directly and remove this ocfs2_swap_dx_entry_list?
> 
> I learn this from ocfs2_swap_dir_entries_direction().
> 
> [snip]
>>>  static void ocfs2_swap_dx_leaf_from_cpu(struct ocfs2_dx_leaf *dx_leaf)
>>>  {
>>>      if (cpu_is_little_endian)
>>>          return;
>>> -    ocfs2_swap_dx_leaf(dx_leaf);
>>> +    ocfs2_swap_dx_leaf(dx_leaf, 0);
>>       dx_leaf->dl_blkno = bswap_64(dx_leaf->dl_blkno);
>>       dx_leaf->dl_fs_generation = bswap_64(dx_leaf->dl_fs_generation);
>>
>>     ocfs2_swap_dx_entry_list_from_cpu(&dx_leaf->dl_list);
>>
>> Or if you don't want the swapping of dl_blkno to display twice, you can
>> add a new function named ocfs2_swap_dx_leaf_primary and add them to it.
>>
> 
> Thanks for your patience. My code is following ocfs2_swap_dir_entries_direction() style, from your comments, it seems
> not a standard code style here. I should follow the de facto style :-)
yeah, actually most of current code don't have a 'to_cpu' paramter.
The reason why ocfs2_swap_dir_entries_direction I guess is that it is 
very complicated.  But your codes isn't the case.
You can check the swap of extent_block, inode etc for reference.

Regards,
Tao
>