[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense
From: |
Alex Bennée |
Subject: |
Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense |
Date: |
Mon, 14 Mar 2022 19:48:47 +0000 |
User-agent: |
mu4e 1.7.10; emacs 28.0.92 |
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).
>
> 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 <armbru@redhat.com>
<snip>
> --- a/audio/jackaudio.c
> +++ b/audio/jackaudio.c
> @@ -97,9 +97,9 @@ static void qjack_buffer_create(QJackBuffer *buffer, int
> channels, int frames)
> buffer->used = 0;
> buffer->rptr = 0;
> buffer->wptr = 0;
> - buffer->data = g_malloc(channels * sizeof(float *));
> + buffer->data = g_new(float *, channels);
> for (int i = 0; i < channels; ++i) {
> - buffer->data[i] = g_malloc(frames * sizeof(float));
> + buffer->data[i] = g_new(float, frames);
Are these actually buffers of pointers to floats? I guess I leave that
to the JACK experts...
> }
> }
>
> @@ -453,7 +453,7 @@ static int qjack_client_init(QJackClient *c)
> jack_on_shutdown(c->client, qjack_shutdown, c);
>
> /* allocate and register the ports */
> - c->port = g_malloc(sizeof(jack_port_t *) * c->nchannels);
> + c->port = g_new(jack_port_t *, c->nchannels);
> for (int i = 0; i < c->nchannels; ++i) {
I guess JACK just likes double indirection...
>
> char port_name[16];
<snip>
> --- 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 **
but I guess the compiler would have complained so maybe I don't
understand the magic enough.
<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.bennee@linaro.org>
--
Alex Bennée
Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense, Eric Blake, 2022/03/15
Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense, Dr. David Alan Gilbert, 2022/03/15