Alex Bennée <alex.ben...@linaro.org> writes:
> Markus Armbruster <arm...@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).
>>
>> Patch created mechanically with:
>>
>> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>> --macro-file scripts/cocci-macro-file.h FILES...
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
[...]
>> --- a/hw/pci/pcie_sriov.c
>> +++ b/hw/pci/pcie_sriov.c
>> @@ -177,7 +177,7 @@ static void register_vfs(PCIDevice *dev)
>> assert(sriov_cap > 0);
>> num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
>>
>> - dev->exp.sriov_pf.vf = g_malloc(sizeof(PCIDevice *) * num_vfs);
>> + dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
>> assert(dev->exp.sriov_pf.vf);
>
> So what I find confusing about the conversion of sizeof(foo *) is that
> while the internal sizeof in g_new is unaffected I think the casting
> ends up as
>
> foo **
Yes. g_malloc(...) returns void *. g_new(T, ...) returns T *.
> but I guess the compiler would have complained so maybe I don't
> understand the magic enough.
It doesn't complain here because dev->exp.sriov_pf.vf is of type
PCIDevice **:
struct PCIESriovPF {
uint16_t num_vfs; /* Number of virtual functions created */
uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */
const char *vfname; /* Reference to the device type used for the VFs */
PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
};
It does complain when the types don't match, as shown in PATCH 2.
> <snip>
>> index 42130667a7..598e6adc5e 100644
>> --- a/hw/rdma/vmw/pvrdma_dev_ring.c
>> +++ b/hw/rdma/vmw/pvrdma_dev_ring.c
>> @@ -41,7 +41,7 @@ int pvrdma_ring_init(PvrdmaRing *ring, const char *name,
>> PCIDevice *dev,
>> qatomic_set(&ring->ring_state->cons_head, 0);
>> */
>> ring->npages = npages;
>> - ring->pages = g_malloc0(npages * sizeof(void *));
>> + ring->pages = g_new0(void *, npages);
>
> At least here ring->pages agrees about void ** being the result.
>
> <snip>
>
> So other than my queries about the sizeof(foo *) which I'd like someone
> to assuage me of my concerns it looks like the script has done a
> thorough mechanical job as all good scripts should ;-)
>
> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
Thanks!
