| Message ID | 20220314160108.1440470-3-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use g_new() & friends where that makes obvious | expand |
Hi Christian, On 14/3/22 17:42, Christian Schoenebeck wrote: > On Montag, 14. März 2022 17:01:07 CET Markus Armbruster 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). >> >> Patch created mechanically with: >> >> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \ >> --macro-file scripts/cocci-macro-file.h FILES... >> >> Except this uncovers a typing error: >> >> ../hw/9pfs/9p.c:855:13: warning: incompatible pointer types assigning to >> 'QpfEntry *' from 'QppEntry *' [-Wincompatible-pointer-types] val = >> g_new0(QppEntry, 1); >> ^ ~~~~~~~~~~~~~~~~~~~ >> 1 warning generated. >> >> Harmless, because QppEntry is larger than QpfEntry. Fix to allocate a >> QpfEntry instead. >> >> Cc: Greg Kurz <groug@kaod.org> >> Cc: Christian Schoenebeck <qemu_oss@crudebyte.com> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- > > Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> FYI your domain is also quarantined by Google: ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@crudebyte.com header.s=lizzy header.b=olij9WvS; spf=softfail (google.com: domain of transitioning qemu_oss@crudebyte.com does not designate 172.105.152.211 as permitted sender) smtp.mailfrom=qemu_oss@crudebyte.com; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=QUARANTINE) header.from=crudebyte.com Received-SPF: softfail (google.com: domain of transitioning qemu_oss@crudebyte.com does not designate 172.105.152.211 as permitted sender) client-ip=172.105.152.211; Authentication-Results: mx.google.com; dkim=fail header.i=@crudebyte.com header.s=lizzy header.b=olij9WvS; spf=softfail (google.com: domain of transitioning qemu_oss@crudebyte.com does not designate 172.105.152.211 as permitted sender) smtp.mailfrom=qemu_oss@crudebyte.com; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=QUARANTINE) header.from=crudebyte.com X-Rspamd-Queue-Id: AC61617709E X-Spamd-Result: default: False [-2.01 / 7.00]; BAYES_HAM(-3.00)[100.00%]; SUSPICIOUS_RECIPS(1.50)[]; DMARC_POLICY_ALLOW(-0.50)[crudebyte.com,quarantine]; MID_RHS_NOT_FQDN(0.50)[]; R_DKIM_ALLOW(-0.20)[crudebyte.com:s=lizzy]; R_SPF_ALLOW(-0.20)[+ip4:91.194.90.13]; MIME_GOOD(-0.10)[text/plain]; MX_GOOD(-0.01)[]; RCVD_COUNT_ZERO(0.00)[0]; ASN(0.00)[asn:51167, ipnet:91.194.90.0/23, country:DE]; MIME_TRACE(0.00)[0:+]; FREEMAIL_CC(0.00)[redhat.com,linaro.org,gmail.com,vger.kernel.org,irrelevant.dk,adacore.com,anisinha.ca,netbsd.org,microsoft.com,kernel.org,lists.xenproject.org,users.sourceforge.jp,xen.org,huawei.com,reactos.org,amd.com,citrix.com,syrmia.com,ilande.co.uk,intel.com,kaod.org,nongnu.org,ispras.ru,gibson.dropbear.id.au,habkost.net,virtuozzo.com,google.com,amsat.org,tribudubois.net,mvista.com]; FROM_EQ_ENVFROM(0.00)[]; NEURAL_HAM(-0.00)[-0.923]; ARC_NA(0.00)[]; DKIM_TRACE(0.00)[crudebyte.com:+]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_GT_50(0.00)[66]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TAGGED_RCPT(0.00)[]; RCVD_IN_DNSWL_FAIL(0.00)[91.194.90.13:server fail] X-Rspamd-Server: atlanta189
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... > > Except this uncovers a typing error: > > ../hw/9pfs/9p.c:855:13: warning: incompatible pointer types assigning to 'QpfEntry *' from 'QppEntry *' [-Wincompatible-pointer-types] > val = g_new0(QppEntry, 1); > ^ ~~~~~~~~~~~~~~~~~~~ > 1 warning generated. > > Harmless, because QppEntry is larger than QpfEntry. Fix to allocate a > QpfEntry instead. > > Cc: Greg Kurz <groug@kaod.org> > Cc: Christian Schoenebeck <qemu_oss@crudebyte.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On Mon, 14 Mar 2022 17:01:07 +0100 Markus Armbruster <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). > > Patch created mechanically with: > > $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \ > --macro-file scripts/cocci-macro-file.h FILES... > > Except this uncovers a typing error: > > ../hw/9pfs/9p.c:855:13: warning: incompatible pointer types assigning to 'QpfEntry *' from 'QppEntry *' [-Wincompatible-pointer-types] > val = g_new0(QppEntry, 1); > ^ ~~~~~~~~~~~~~~~~~~~ > 1 warning generated. > > Harmless, because QppEntry is larger than QpfEntry. Fix to allocate a > QpfEntry instead. > > Cc: Greg Kurz <groug@kaod.org> > Cc: Christian Schoenebeck <qemu_oss@crudebyte.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > hw/9pfs/9p-proxy.c | 2 +- > hw/9pfs/9p-synth.c | 4 ++-- > hw/9pfs/9p.c | 8 ++++---- > hw/9pfs/codir.c | 6 +++--- > tests/qtest/virtio-9p-test.c | 4 ++-- > 5 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index 8b4b5cf7dc..4c5e0fc217 100644 > --- a/hw/9pfs/9p-proxy.c > +++ b/hw/9pfs/9p-proxy.c > @@ -1187,7 +1187,7 @@ static int proxy_parse_opts(QemuOpts *opts, FsDriverEntry *fs, Error **errp) > > static int proxy_init(FsContext *ctx, Error **errp) > { > - V9fsProxy *proxy = g_malloc(sizeof(V9fsProxy)); > + V9fsProxy *proxy = g_new(V9fsProxy, 1); > int sock_id; > > if (ctx->export_flags & V9FS_PROXY_SOCK_NAME) { > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c > index b3080e415b..d99d263985 100644 > --- a/hw/9pfs/9p-synth.c > +++ b/hw/9pfs/9p-synth.c > @@ -49,7 +49,7 @@ static V9fsSynthNode *v9fs_add_dir_node(V9fsSynthNode *parent, int mode, > > /* Add directory type and remove write bits */ > mode = ((mode & 0777) | S_IFDIR) & ~(S_IWUSR | S_IWGRP | S_IWOTH); > - node = g_malloc0(sizeof(V9fsSynthNode)); > + node = g_new0(V9fsSynthNode, 1); > if (attr) { > /* We are adding .. or . entries */ > node->attr = attr; > @@ -128,7 +128,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode, > } > /* Add file type and remove write bits */ > mode = ((mode & 0777) | S_IFREG); > - node = g_malloc0(sizeof(V9fsSynthNode)); > + node = g_new0(V9fsSynthNode, 1); > node->attr = &node->actual_attr; > node->attr->inode = synth_node_count++; > node->attr->nlink = 1; > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index a6d6b3f835..8e9d4aea73 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -324,7 +324,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) > return NULL; > } > } > - f = g_malloc0(sizeof(V9fsFidState)); > + f = g_new0(V9fsFidState, 1); > f->fid = fid; > f->fid_type = P9_FID_NONE; > f->ref = 1; > @@ -804,7 +804,7 @@ static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t dev) > > val = qht_lookup(&pdu->s->qpd_table, &lookup, hash); > if (!val) { > - val = g_malloc0(sizeof(QpdEntry)); > + val = g_new0(QpdEntry, 1); > *val = lookup; > affix = affixForIndex(pdu->s->qp_affix_next); > val->prefix_bits = affix.bits; > @@ -852,7 +852,7 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf, > return -ENFILE; > } > > - val = g_malloc0(sizeof(QppEntry)); > + val = g_new0(QpfEntry, 1); > *val = lookup; > > /* new unique inode and device combo */ > @@ -928,7 +928,7 @@ static int qid_path_suffixmap(V9fsPDU *pdu, const struct stat *stbuf, > return -ENFILE; > } > > - val = g_malloc0(sizeof(QppEntry)); > + val = g_new0(QppEntry, 1); > *val = lookup; > > /* new unique inode affix and device combo */ > diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c > index 75148bc985..93ba44fb75 100644 > --- a/hw/9pfs/codir.c > +++ b/hw/9pfs/codir.c > @@ -141,9 +141,9 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, > > /* append next node to result chain */ > if (!e) { > - *entries = e = g_malloc0(sizeof(V9fsDirEnt)); > + *entries = e = g_new0(V9fsDirEnt, 1); > } else { > - e = e->next = g_malloc0(sizeof(V9fsDirEnt)); > + e = e->next = g_new0(V9fsDirEnt, 1); > } > e->dent = qemu_dirent_dup(dent); > > @@ -163,7 +163,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, > break; > } > > - e->st = g_malloc0(sizeof(struct stat)); > + e->st = g_new0(struct stat, 1); > memcpy(e->st, &stbuf, sizeof(struct stat)); > } > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c > index 01ca076afe..e28c71bd8f 100644 > --- a/tests/qtest/virtio-9p-test.c > +++ b/tests/qtest/virtio-9p-test.c > @@ -468,12 +468,12 @@ static void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, > togo -= 13 + 8 + 1 + 2 + slen, ++n) > { > if (!e) { > - e = g_malloc(sizeof(struct V9fsDirent)); > + e = g_new(struct V9fsDirent, 1); > if (entries) { > *entries = e; > } > } else { > - e = e->next = g_malloc(sizeof(struct V9fsDirent)); > + e = e->next = g_new(struct V9fsDirent, 1); > } > e->next = NULL; > /* qid[13] offset[8] type[1] name[s] */
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c index 8b4b5cf7dc..4c5e0fc217 100644 --- a/hw/9pfs/9p-proxy.c +++ b/hw/9pfs/9p-proxy.c @@ -1187,7 +1187,7 @@ static int proxy_parse_opts(QemuOpts *opts, FsDriverEntry *fs, Error **errp) static int proxy_init(FsContext *ctx, Error **errp) { - V9fsProxy *proxy = g_malloc(sizeof(V9fsProxy)); + V9fsProxy *proxy = g_new(V9fsProxy, 1); int sock_id; if (ctx->export_flags & V9FS_PROXY_SOCK_NAME) { diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c index b3080e415b..d99d263985 100644 --- a/hw/9pfs/9p-synth.c +++ b/hw/9pfs/9p-synth.c @@ -49,7 +49,7 @@ static V9fsSynthNode *v9fs_add_dir_node(V9fsSynthNode *parent, int mode, /* Add directory type and remove write bits */ mode = ((mode & 0777) | S_IFDIR) & ~(S_IWUSR | S_IWGRP | S_IWOTH); - node = g_malloc0(sizeof(V9fsSynthNode)); + node = g_new0(V9fsSynthNode, 1); if (attr) { /* We are adding .. or . entries */ node->attr = attr; @@ -128,7 +128,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode, } /* Add file type and remove write bits */ mode = ((mode & 0777) | S_IFREG); - node = g_malloc0(sizeof(V9fsSynthNode)); + node = g_new0(V9fsSynthNode, 1); node->attr = &node->actual_attr; node->attr->inode = synth_node_count++; node->attr->nlink = 1; diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index a6d6b3f835..8e9d4aea73 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -324,7 +324,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) return NULL; } } - f = g_malloc0(sizeof(V9fsFidState)); + f = g_new0(V9fsFidState, 1); f->fid = fid; f->fid_type = P9_FID_NONE; f->ref = 1; @@ -804,7 +804,7 @@ static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t dev) val = qht_lookup(&pdu->s->qpd_table, &lookup, hash); if (!val) { - val = g_malloc0(sizeof(QpdEntry)); + val = g_new0(QpdEntry, 1); *val = lookup; affix = affixForIndex(pdu->s->qp_affix_next); val->prefix_bits = affix.bits; @@ -852,7 +852,7 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf, return -ENFILE; } - val = g_malloc0(sizeof(QppEntry)); + val = g_new0(QpfEntry, 1); *val = lookup; /* new unique inode and device combo */ @@ -928,7 +928,7 @@ static int qid_path_suffixmap(V9fsPDU *pdu, const struct stat *stbuf, return -ENFILE; } - val = g_malloc0(sizeof(QppEntry)); + val = g_new0(QppEntry, 1); *val = lookup; /* new unique inode affix and device combo */ diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c index 75148bc985..93ba44fb75 100644 --- a/hw/9pfs/codir.c +++ b/hw/9pfs/codir.c @@ -141,9 +141,9 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, /* append next node to result chain */ if (!e) { - *entries = e = g_malloc0(sizeof(V9fsDirEnt)); + *entries = e = g_new0(V9fsDirEnt, 1); } else { - e = e->next = g_malloc0(sizeof(V9fsDirEnt)); + e = e->next = g_new0(V9fsDirEnt, 1); } e->dent = qemu_dirent_dup(dent); @@ -163,7 +163,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, break; } - e->st = g_malloc0(sizeof(struct stat)); + e->st = g_new0(struct stat, 1); memcpy(e->st, &stbuf, sizeof(struct stat)); } diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 01ca076afe..e28c71bd8f 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -468,12 +468,12 @@ static void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, togo -= 13 + 8 + 1 + 2 + slen, ++n) { if (!e) { - e = g_malloc(sizeof(struct V9fsDirent)); + e = g_new(struct V9fsDirent, 1); if (entries) { *entries = e; } } else { - e = e->next = g_malloc(sizeof(struct V9fsDirent)); + e = e->next = g_new(struct V9fsDirent, 1); } e->next = NULL; /* qid[13] offset[8] type[1] name[s] */
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... Except this uncovers a typing error: ../hw/9pfs/9p.c:855:13: warning: incompatible pointer types assigning to 'QpfEntry *' from 'QppEntry *' [-Wincompatible-pointer-types] val = g_new0(QppEntry, 1); ^ ~~~~~~~~~~~~~~~~~~~ 1 warning generated. Harmless, because QppEntry is larger than QpfEntry. Fix to allocate a QpfEntry instead. Cc: Greg Kurz <groug@kaod.org> Cc: Christian Schoenebeck <qemu_oss@crudebyte.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/9pfs/9p-proxy.c | 2 +- hw/9pfs/9p-synth.c | 4 ++-- hw/9pfs/9p.c | 8 ++++---- hw/9pfs/codir.c | 6 +++--- tests/qtest/virtio-9p-test.c | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-)