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

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

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 30.05.2014 18:30, 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/sheepdog.c    | 2 +-
>>   block/vhdx-log.c    | 2 +-
>>   block/vvfat.c       | 8 ++++----
>>   hw/ide/microdrive.c | 2 +-
>>   4 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index 0709fd0..340bdf6 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -2202,7 +2202,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
>>           goto cleanup;
>>       }
>>   -    inode = (SheepdogInode *)g_malloc(datalen);
>> +    inode = g_malloc(datalen);
>>         ret = read_object(fd, (char *)inode,
>> vid_to_vdi_oid(new_vid),
>>                         s->inode.nr_copies, datalen, 0, s->cache_flags);
>> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
>> index 3eb7e68..f07328d 100644
>> --- a/block/vhdx-log.c
>> +++ b/block/vhdx-log.c
>> @@ -909,7 +909,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);
>
> Perhaps we should try to fix void pointer arithmetic some time...

Fixes for that tend to involve back-and-forth type casting.  I find that
cure not much of an improvement over the disease :)

> Anyway:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks!

Re: [Qemu-devel] [PATCH 0/3] block: Use g_new() & friends more

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> This series is on top of Kevin's "block: Handle failure for
> potentially large allocations".
>
> PATCH 1+2 convert some allocations.  While preparing them, I stumbled
> over some useless casts, which led to PATCH 3.

Fell through the cracks despite Max's R-by, and now it doesn't apply
anymore.  Pity, because it improves type safety and reduces
vulnerability to integer overflows.

Is this change wanted?  Unless I get a "yes" from a maintainer, I'll
drop it.

> I posted a tree-wide version of PATCH 1 some time ago, and was told to
> split it up.  This is the block part, redone from scratch.  Can't say
> when (or even whether) I can do more parts.

Probably not.  Even I learn from experience eventually.

Re: [Qemu-devel] [PATCH 0/3] block: Use g_new() & friends more

[Kevin Wolf]

Am 01.07.2014 um 11:06 hat Markus Armbruster geschrieben:
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > This series is on top of Kevin's "block: Handle failure for
> > potentially large allocations".
> >
> > PATCH 1+2 convert some allocations.  While preparing them, I stumbled
> > over some useless casts, which led to PATCH 3.
> 
> Fell through the cracks despite Max's R-by, and now it doesn't apply
> anymore.  Pity, because it improves type safety and reduces
> vulnerability to integer overflows.

Whoops. Not sure if Stefan had any reasons not to merge it (the tree was
his for three weeks because I was away in mine), but it probably really
just fell through the cracks.

> Is this change wanted?  Unless I get a "yes" from a maintainer, I'll
> drop it.

Yes, I think it's a good change.

Kevin

Re: [Qemu-devel] [PATCH 0/3] block: Use g_new() & friends more

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 01.07.2014 um 11:06 hat Markus Armbruster geschrieben:
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>> > This series is on top of Kevin's "block: Handle failure for
>> > potentially large allocations".
>> >
>> > PATCH 1+2 convert some allocations.  While preparing them, I stumbled
>> > over some useless casts, which led to PATCH 3.
>> 
>> Fell through the cracks despite Max's R-by, and now it doesn't apply
>> anymore.  Pity, because it improves type safety and reduces
>> vulnerability to integer overflows.
>
> Whoops. Not sure if Stefan had any reasons not to merge it (the tree was
> his for three weeks because I was away in mine), but it probably really
> just fell through the cracks.

Happens :)  I should've complained earlier.

>> Is this change wanted?  Unless I get a "yes" from a maintainer, I'll
>> drop it.
>
> Yes, I think it's a good change.

Okay, I'll redo the series for 2.2.