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

[Tao Ma]

Hi coly,

Coly Li wrote:
> As Tao Ma suggested, current ocfs2_swap_dx_entry_list() is
> buggy for big endian hardware, because after dl_list->de_count
> swapped, it is referenced in the following loop.
> 
> This patch fixes this bug with adding an 'int to_cpu' argument, also
> modifies other routines who call ocfs2_swap_dx_entry_list().
> 
> Signed-off-by: Coly Li <coly.li at suse.de>
> Cc: Mark Fasheh <mfasheh at suse.com>
> Cc: Tao Ma <tao.ma at oracle.com>
> ---
>  libocfs2/dirblock.c |   23 ++++++++++++++---------
>  1 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/libocfs2/dirblock.c b/libocfs2/dirblock.c
> index c22d843..e128d73 100644
> --- a/libocfs2/dirblock.c
> +++ b/libocfs2/dirblock.c
> @@ -245,29 +245,34 @@ static void ocfs2_swap_dx_entry(struct ocfs2_dx_entry *dx_entry)
>  	dx_entry->dx_dirent_blk		= bswap_64(dx_entry->dx_dirent_blk);
>  }
> 
> -static void ocfs2_swap_dx_entry_list(struct ocfs2_dx_entry_list *dl_list)
> +/* 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?
> 
>  static void ocfs2_swap_dx_entry_list_to_cpu(struct ocfs2_dx_entry_list *dl_list)
>  {
>  	if (cpu_is_little_endian)
>  		return;
> -	ocfs2_swap_dx_entry_list(dl_list);
> +	ocfs2_swap_dx_entry_list(dl_list, 1);
Here we do:
	dl_list->de_count = bswap_16(dl_list->de_count);
	dl_list->de_num_used = bswap_16(dl_list->de_num_used);

  	for (i = 0; i < dl_list->de_count; i++)
  		ocfs2_swap_dx_entry(&dl_list->de_entries[i]);

>  }
> 
>  static void ocfs2_swap_dx_entry_list_from_cpu(struct ocfs2_dx_entry_list *dl_list)
>  {
>  	if (cpu_is_little_endian)
>  		return;
> -	ocfs2_swap_dx_entry_list(dl_list);
> +	ocfs2_swap_dx_entry_list(dl_list, 0);
Here we do:
  	for (i = 0; i < dl_list->de_count; i++)
  		ocfs2_swap_dx_entry(&dl_list->de_entries[i]);
	dl_list->de_count = bswap_16(dl_list->de_count);
	dl_list->de_num_used = bswap_16(dl_list->de_num_used);

>  }
> 
>  static void ocfs2_swap_dx_root_to_cpu(ocfs2_filesys *fs,
> @@ -384,26 +389,26 @@ out:
>  	return ret;
>  }
> 
> -static void ocfs2_swap_dx_leaf(struct ocfs2_dx_leaf *dx_leaf)
> +static void ocfs2_swap_dx_leaf(struct ocfs2_dx_leaf *dx_leaf, int to_cpu)
>  {
>  	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(&dx_leaf->dl_list);
> +	ocfs2_swap_dx_entry_list(&dx_leaf->dl_list, to_cpu);
>  }
Remove this function.
> 
>  static void ocfs2_swap_dx_leaf_to_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, 1);
   	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_to_cpu(&dx_leaf->dl_list);

>  }
> 
>  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.

Regards,
Tao