* Markus Armbruster (arm...@redhat.com) wrote:
> Philippe Mathieu-Daudé <f4...@amsat.org> writes:
>
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> > [PMD: more changes]
> > ---
> > monitor.c | 14 +++++++-------
> > qmp.c | 14 +++++++-------
> > tests/test-qmp-commands.c | 14 +++++++-------
> > 3 files changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index fe0d1bdbb4..ea6a485f11 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -906,7 +906,7 @@ static void query_commands_cb(QmpCommand *cmd, void
> > *opaque)
> > return;
> > }
> >
> > - info = g_malloc0(sizeof(*info));
> > + info = g_new0(CommandInfoList, 1);
> > info->value = g_malloc0(sizeof(*info->value));
> > info->value->name = g_strdup(cmd->name);
> > info->next = *list;
>
> I'm not convinced rewriting
>
> LHS = g_malloc(sizeof(*LHS));
>
> to
>
> LHS = g_new(T, 1);
>
> where T is the type of LHS is worth the trouble. The code before the
> rewrite is pretty idiomatic. There's no possibility of integer overflow
> g_new() could avoid. The types are obviously correct, so the additional
> type checking is quite unlikely to catch anything. That leaves the
> consistency argument. I'm willing to hear it, but I feel it should be
> heard in a patch series that does nothing else.
The 'obviously correct' is the dodgy part of the argument here.
How many bugs do we right that are obviously wrong?
t.c:13:20: warning: initialization from incompatible pointer type
[-Wincompatible-pointer-types]
struct c *pc = g_new(struct b, 1);
seems good to me.
Dave
> > @@ -1799,7 +1799,7 @@ static void hmp_wavcapture(Monitor *mon, const QDict
> > *qdict)
> > int nchannels = qdict_get_try_int(qdict, "nchannels", -1);
> > CaptureState *s;
> >
> > - s = g_malloc0 (sizeof (*s));
> > + s = g_new0(CaptureState, 1);
> >
> > freq = has_freq ? freq : 44100;
> > bits = has_bits ? bits : 16;
>
> This hunk is HMP, not QMP.
>
> > @@ -1947,7 +1947,7 @@ void qmp_getfd(const char *fdname, Error **errp)
> > return;
> > }
> >
> > - monfd = g_malloc0(sizeof(mon_fd_t));
> > + monfd = g_new0(mon_fd_t, 1);
> > monfd->name = g_strdup(fdname);
> > monfd->fd = fd;
> >
> > @@ -2110,7 +2110,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
> > QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
> > FdsetFdInfoList *fdsetfd_info;
> >
> > - fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info));
> > + fdsetfd_info = g_new0(FdsetFdInfoList, 1);
> > fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value));
> > fdsetfd_info->value->fd = mon_fdset_fd->fd;
> > if (mon_fdset_fd->opaque) {
> > @@ -2199,7 +2199,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool
> > has_fdset_id, int64_t fdset_id,
> > }
> > }
> >
> > - mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd));
> > + mon_fdset_fd = g_new0(MonFdsetFd, 1);
> > mon_fdset_fd->fd = fd;
> > mon_fdset_fd->removed = false;
> > if (has_opaque) {
> > @@ -2207,7 +2207,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool
> > has_fdset_id, int64_t fdset_id,
> > }
> > QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next);
> >
> > - fdinfo = g_malloc0(sizeof(*fdinfo));
> > + fdinfo = g_new0(AddfdInfo, 1);
> > fdinfo->fdset_id = mon_fdset->id;
> > fdinfo->fd = mon_fdset_fd->fd;
> >
> > @@ -4102,7 +4102,7 @@ void monitor_init(Chardev *chr, int flags)
> > is_first_init = 0;
> > }
> >
> > - mon = g_malloc(sizeof(*mon));
> > + mon = g_new(Monitor, 1);
> > monitor_data_init(mon);
> >
> > qemu_chr_fe_init(&mon->chr, chr, &error_abort);
>
> This is monitor core, not QMP.
>
> > diff --git a/qmp.c b/qmp.c
> > index e8c303116a..e965020e37 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -232,7 +232,7 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path,
> > Error **errp)
> > while ((prop = object_property_iter_next(&iter))) {
> > ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry));
> >
> > - entry->value = g_malloc0(sizeof(ObjectPropertyInfo));
> > + entry->value = g_new0(ObjectPropertyInfo, 1);
> > entry->next = props;
> > props = entry;
> >
> > @@ -432,7 +432,7 @@ static void qom_list_types_tramp(ObjectClass *klass,
> > void *data)
> > ObjectTypeInfo *info;
> > ObjectClass *parent = object_class_get_parent(klass);
> >
> > - info = g_malloc0(sizeof(*info));
> > + info = g_new0(ObjectTypeInfo, 1);
> > info->name = g_strdup(object_class_get_name(klass));
> > info->has_abstract = info->abstract = object_class_is_abstract(klass);
> > if (parent) {
> > @@ -440,7 +440,7 @@ static void qom_list_types_tramp(ObjectClass *klass,
> > void *data)
> > info->parent = g_strdup(object_class_get_name(parent));
> > }
> >
> > - e = g_malloc0(sizeof(*e));
> > + e = g_new0(ObjectTypeInfoList, 1);
> > e->value = info;
> > e->next = *pret;
> > *pret = e;
> > @@ -490,7 +490,7 @@ static DevicePropertyInfo
> > *make_device_property_info(ObjectClass *klass,
> > return NULL; /* no way to set it, don't show */
> > }
> >
> > - info = g_malloc0(sizeof(*info));
> > + info = g_new0(DevicePropertyInfo, 1);
> > info->name = g_strdup(prop->name);
> > info->type = default_type ? g_strdup(default_type)
> > : g_strdup(prop->info->name);
> > @@ -502,7 +502,7 @@ static DevicePropertyInfo
> > *make_device_property_info(ObjectClass *klass,
> > } while (klass != object_class_by_name(TYPE_DEVICE));
> >
> > /* Not a qdev property, use the default type */
> > - info = g_malloc0(sizeof(*info));
> > + info = g_new0(DevicePropertyInfo, 1);
> > info->name = g_strdup(name);
> > info->type = g_strdup(default_type);
> > info->has_description = !!description;
> > @@ -568,7 +568,7 @@ DevicePropertyInfoList
> > *qmp_device_list_properties(const char *typename,
> > continue;
> > }
> >
> > - entry = g_malloc0(sizeof(*entry));
> > + entry = g_new0(DevicePropertyInfoList, 1);
> > entry->value = info;
> > entry->next = prop_list;
> > prop_list = entry;
> > @@ -712,7 +712,7 @@ ACPIOSTInfoList *qmp_query_acpi_ospm_status(Error
> > **errp)
> >
> > MemoryInfo *qmp_query_memory_size_summary(Error **errp)
> > {
> > - MemoryInfo *mem_info = g_malloc0(sizeof(MemoryInfo));
> > + MemoryInfo *mem_info = g_new0(MemoryInfo, 1);
> >
> > mem_info->base_memory = ram_size;
> >
> > diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> > index 904c89d4d4..e715c45a23 100644
> > --- a/tests/test-qmp-commands.c
> > +++ b/tests/test-qmp-commands.c
> > @@ -28,8 +28,8 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
> > Error **errp)
> > {
> > UserDefTwo *ret;
> > - UserDefOne *ud1c = g_malloc0(sizeof(UserDefOne));
> > - UserDefOne *ud1d = g_malloc0(sizeof(UserDefOne));
> > + UserDefOne *ud1c = g_new0(UserDefOne, 1);
> > + UserDefOne *ud1d = g_new0(UserDefOne, 1);
> >
> > ud1c->string = strdup(ud1a->string);
> > ud1c->integer = ud1a->integer;
> > @@ -209,23 +209,23 @@ static void test_dealloc_types(void)
> > UserDefOne *ud1test, *ud1a, *ud1b;
> > UserDefOneList *ud1list;
> >
> > - ud1test = g_malloc0(sizeof(UserDefOne));
> > + ud1test = g_new0(UserDefOne, 1);
> > ud1test->integer = 42;
> > ud1test->string = g_strdup("hi there 42");
> >
> > qapi_free_UserDefOne(ud1test);
> >
> > - ud1a = g_malloc0(sizeof(UserDefOne));
> > + ud1a = g_new0(UserDefOne, 1);
> > ud1a->integer = 43;
> > ud1a->string = g_strdup("hi there 43");
> >
> > - ud1b = g_malloc0(sizeof(UserDefOne));
> > + ud1b = g_new0(UserDefOne, 1);
> > ud1b->integer = 44;
> > ud1b->string = g_strdup("hi there 44");
> >
> > - ud1list = g_malloc0(sizeof(UserDefOneList));
> > + ud1list = g_new0(UserDefOneList, 1);
> > ud1list->value = ud1a;
> > - ud1list->next = g_malloc0(sizeof(UserDefOneList));
> > + ud1list->next = g_new0(UserDefOneList, 1);
> > ud1list->next->value = ud1b;
> >
> > qapi_free_UserDefOneList(ud1list);
>
> Could squash together with PATCH 79 and call the result "monitor: Use
> g_new() family of functions".
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
