[Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.

This commit only touches allocations with size arguments of the form
sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/rocker/rocker.c        | 2 +-
 hw/net/rocker/rocker_desc.c   | 4 ++--
 hw/net/rocker/rocker_fp.c     | 2 +-
 hw/net/rocker/rocker_of_dpa.c | 9 ++++-----
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 7e7bda4..bb6fdc3 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1362,7 +1362,7 @@ static int pci_rocker_init(PCIDevice *dev)
         r->fp_ports = ROCKER_FP_PORTS_MAX;
     }
 
-    r->rings = g_malloc(sizeof(DescRing *) * rocker_pci_ring_count(r));
+    r->rings = g_new(DescRing *, rocker_pci_ring_count(r));
     if (!r->rings) {
         goto err_rings_alloc;
     }
diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
index b5c0b4a..5e697b1 100644
--- a/hw/net/rocker/rocker_desc.c
+++ b/hw/net/rocker/rocker_desc.c
@@ -142,7 +142,7 @@ bool desc_ring_set_size(DescRing *ring, uint32_t size)
     ring->size = size;
     ring->head = ring->tail = 0;
 
-    ring->info = g_realloc(ring->info, size * sizeof(DescInfo));
+    ring->info = g_renew(DescInfo, ring->info, size);
     if (!ring->info) {
         return false;
     }
@@ -345,7 +345,7 @@ DescRing *desc_ring_alloc(Rocker *r, int index)
 {
     DescRing *ring;
 
-    ring = g_malloc0(sizeof(DescRing));
+    ring = g_new0(DescRing, 1);
     if (!ring) {
         return NULL;
     }
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index c693ae5..5906396 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -218,7 +218,7 @@ FpPort *fp_port_alloc(Rocker *r, char *sw_name,
                       MACAddr *start_mac, unsigned int index,
                       NICPeers *peers)
 {
-    FpPort *port = g_malloc0(sizeof(FpPort));
+    FpPort *port = g_new0(FpPort, 1);
 
     if (!port) {
         return NULL;
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index 874fb01..1ad2791 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -367,7 +367,7 @@ static OfDpaFlow *of_dpa_flow_alloc(uint64_t cookie)
     OfDpaFlow *flow;
     int64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) / 1000;
 
-    flow = g_malloc0(sizeof(OfDpaFlow));
+    flow = g_new0(OfDpaFlow, 1);
     if (!flow) {
         return NULL;
     }
@@ -811,7 +811,7 @@ static int of_dpa_group_get_stats(OfDpa *of_dpa, uint32_t id)
 
 static OfDpaGroup *of_dpa_group_alloc(uint32_t id)
 {
-    OfDpaGroup *group = g_malloc0(sizeof(OfDpaGroup));
+    OfDpaGroup *group = g_new0(OfDpaGroup, 1);
 
     if (!group) {
         return NULL;
@@ -2039,15 +2039,14 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa, OfDpaGroup *group,
     group->l2_flood.group_count =
         rocker_tlv_get_le16(group_tlvs[ROCKER_TLV_OF_DPA_GROUP_COUNT]);
 
-    tlvs = g_malloc0((group->l2_flood.group_count + 1) *
-                     sizeof(RockerTlv *));
+    tlvs = g_new0(RockerTlv *, group->l2_flood.group_count + 1);
     if (!tlvs) {
         return -ROCKER_ENOMEM;
     }
 
     g_free(group->l2_flood.group_ids);
     group->l2_flood.group_ids =
-        g_malloc0(group->l2_flood.group_count * sizeof(uint32_t));
+        g_new0(uint32_t, group->l2_flood.group_count);
     if (!group->l2_flood.group_ids) {
         err = -ROCKER_ENOMEM;
         goto err_out;
-- 
2.4.3

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Jiri Pirko]

Mon, Sep 14, 2015 at 01:52:23PM CEST, armbru@redhat.com wrote:
>g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>for two reasons.  One, it catches multiplication overflowing size_t.
>Two, it returns T * rather than void *, which lets the compiler catch
>more type errors.
>
>This commit only touches allocations with size arguments of the form
>sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.

                                           ^ typo :)


Other than that:

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On 09/14/2015 05:57 AM, Jiri Pirko wrote:
> Mon, Sep 14, 2015 at 01:52:23PM CEST, armbru@redhat.com wrote:
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>> for two reasons.  One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.
>>
>> This commit only touches allocations with size arguments of the form
>> sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.
> 
>                                            ^ typo :)
> 

This typo is copy-pasted into ALL of your recent g_new() cleanups. Since
you did scattershot threads across multiple maintainers rather than one
big thread, it may be a bit harder to plug all the instances before they
get pulled through the various trees.

> 
> Other than that:
> 
> Acked-by: Jiri Pirko <jiri@resnulli.us>

Reviewed-by: Eric Blake <eblake@redhat.com>

[In qemu, we tend to use 'Reviewed-by' for "I've inspected the code and
agree it correctly does what the commit message claims", and the weaker
'Acked-by' for "I agree with the fix as documented in the commit message
but didn't inspect the code to ensure that they match"]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 980 bytes --]

On 09/14/2015 09:55 AM, Eric Blake wrote:
> On 09/14/2015 05:57 AM, Jiri Pirko wrote:
>> Mon, Sep 14, 2015 at 01:52:23PM CEST, armbru@redhat.com wrote:
>>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>>> for two reasons.  One, it catches multiplication overflowing size_t.
>>> Two, it returns T * rather than void *, which lets the compiler catch
>>> more type errors.
>>>
>>> This commit only touches allocations with size arguments of the form
>>> sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.
>>
>>                                            ^ typo :)
>>
> 
> This typo is copy-pasted into ALL of your recent g_new() cleanups.

Actually, not all of them.

And when I first read your report of the typo, I was thinking that it
meant 'patches', although now I see that it should be two words 'patch as'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 09/14/2015 05:57 AM, Jiri Pirko wrote:
>> Mon, Sep 14, 2015 at 01:52:23PM CEST, armbru@redhat.com wrote:
>>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>>> for two reasons.  One, it catches multiplication overflowing size_t.
>>> Two, it returns T * rather than void *, which lets the compiler catch
>>> more type errors.
>>>
>>> This commit only touches allocations with size arguments of the form
>>> sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.
>> 
>>                                            ^ typo :)
>> 
>
> This typo is copy-pasted into ALL of your recent g_new() cleanups. Since
> you did scattershot threads across multiple maintainers rather than one
> big thread, it may be a bit harder to plug all the instances before they
> get pulled through the various trees.

Only three of them, actually: this one, linux-user and qemu-char.

[...]

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Jiri Pirko]

Mon, Sep 14, 2015 at 05:55:40PM CEST, eblake@redhat.com wrote:
>On 09/14/2015 05:57 AM, Jiri Pirko wrote:
>> Mon, Sep 14, 2015 at 01:52:23PM CEST, armbru@redhat.com wrote:
>>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>>> for two reasons.  One, it catches multiplication overflowing size_t.
>>> Two, it returns T * rather than void *, which lets the compiler catch
>>> more type errors.
>>>
>>> This commit only touches allocations with size arguments of the form
>>> sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.
>> 
>>                                            ^ typo :)
>> 
>
>This typo is copy-pasted into ALL of your recent g_new() cleanups. Since
>you did scattershot threads across multiple maintainers rather than one
>big thread, it may be a bit harder to plug all the instances before they
>get pulled through the various trees.
>
>> 
>> Other than that:
>> 
>> Acked-by: Jiri Pirko <jiri@resnulli.us>
>
>Reviewed-by: Eric Blake <eblake@redhat.com>
>
>[In qemu, we tend to use 'Reviewed-by' for "I've inspected the code and
>agree it correctly does what the commit message claims", and the weaker
>'Acked-by' for "I agree with the fix as documented in the commit message
>but didn't inspect the code to ensure that they match"]

Reviewed-by: Jiri Pirko <jiri@mellanox.com>

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Michael, could you take this one through trivial?  Assuming Scott and
Jiri don't mind, and with s/patchas/patch as/ in the commit message.

Markus Armbruster <armbru@redhat.com> writes:

> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> for two reasons.  One, it catches multiplication overflowing size_t.
> Two, it returns T * rather than void *, which lets the compiler catch
> more type errors.
>
> This commit only touches allocations with size arguments of the form
> sizeof(T).  Same Coccinelle semantic patchas in commit b45c03f.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Jiri Pirko]

Thu, Sep 24, 2015 at 06:18:43PM CEST, armbru@redhat.com wrote:
>Michael, could you take this one through trivial?  Assuming Scott and
>Jiri don't mind, and with s/patchas/patch as/ in the commit message.

I don't mind :)

Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense

[Michael Tokarev]

24.09.2015 19:18, Markus Armbruster wrote:
> Michael, could you take this one through trivial?  Assuming Scott and
> Jiri don't mind, and with s/patchas/patch as/ in the commit message.
> 
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>> for two reasons.  One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.

Applied to -trivial, thanks!

/mjt