Re: [PATCH 2/3] 9pfs: Use g_new() & friends where that makes obvious sense

[Christian Schoenebeck]

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>

>  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] */