Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *

[Jeff Cody <jcody@redhat.com>]

On Mon, Aug 18, 2014 at 06:10:43PM +0200, Markus Armbruster wrote:
> They clutter the code.  Unfortunately, I can't figure out how to make
> Coccinelle drop all of them, so I have to settle for common special
> cases:
> 
>     @@
>     type T;
>     T *pt;
>     void *pv;
>     @@
>     - pt = (T *)pv;
>     + pt = pv;
>     @@
>     type T;
>     @@
>     - (T *)
>       (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\|
> 	 g_try_malloc\|g_try_malloc0\|g_try_realloc\|
> 	 g_try_new\|g_try_new0\|g_try_renew\)(...))
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vhdx-log.c    | 2 +-
>  block/vvfat.c       | 8 ++++----
>  hw/ide/microdrive.c | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
> index eb5c7a0..4267e60 100644
> --- a/block/vhdx-log.c
> +++ b/block/vhdx-log.c
> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s,
>      buffer = qemu_blockalign(bs, total_length);
>      memcpy(buffer, &new_hdr, sizeof(new_hdr));
>  
> -    new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr));
> +    new_desc = (buffer + sizeof(new_hdr));

Agree with Max, in that the parenthesis could be removed.  Not worthy
of a respin normally, but since the point of this patch is to
unclutter the code, I guess it makes sense to fix it.

>      data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE);
>      data_tmp = data;
>  
> diff --git a/block/vvfat.c b/block/vvfat.c
> index f877e85..401539d 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index)
>  	if(first_cluster == 0 && (is_dotdot || is_dot))
>  	    continue;
>  
> -	buffer=(char*)g_malloc(length);
> +	buffer=g_malloc(length);

You missed a spot to put spaces around the '=' here.  Nothing worthy
of a respin, of course - just a note in case you decide to respin.


[...]

With or without the changes above:

Reviewed-by: Jeff Cody <jcody@redhat.com>