On 15.05.2025 11:58, Daniel P. Berrangé wrote:
> On Wed, Apr 30, 2025 at 07:27:29AM +0200, Mario Fleischmann wrote:
>> This commit implements the necessary operations required to establish
>> a connection with the MCD server:
>>
>> * query information about the server
>> * connect to "
>> * disconnect from "
>>
>> Signed-off-by: Mario Fleischmann <mario.fleischm...@lauterbach.com>
>> ---
>> mcd/mcd_qapi.c | 13 +++
>> mcd/mcd_qapi.h | 2 +
>> mcd/mcd_server.c | 110 +++++++++++++++++++++-
>> mcd/mcd_stub.c | 98 ++++++++++++++++++++
>> qapi/mcd.json | 205 +++++++++++++++++++++++++++++++++++++++++
>> tests/qtest/mcd-test.c | 96 +++++++++++++++++++
>> tests/qtest/mcd-util.c | 60 ++++++++++++
>> tests/qtest/mcd-util.h | 9 ++
>> 8 files changed, 588 insertions(+), 5 deletions(-)
>>
>> diff --git a/mcd/mcd_qapi.c b/mcd/mcd_qapi.c
>> index 9a99866..d2a2926 100644
>> --- a/mcd/mcd_qapi.c
>> +++ b/mcd/mcd_qapi.c
>
>
>> +MCDQryServersResult *qmp_mcd_qry_servers(const char *host, bool running,
>> + uint32_t start_index,
>> + uint32_t num_servers, Error **errp)
>> +{
>> + MCDServerInfoList **tailp;
>> + MCDServerInfo *info;
>> + mcd_server_info_st *server_info = NULL;
>> + bool query_num_only = num_servers == 0;
>> + MCDQryServersResult *result = g_malloc0(sizeof(*result));
>> +
>> + if (!query_num_only) {
>> + server_info = g_malloc0(num_servers * sizeof(*server_info));
>
> This multiplication is (theoretically) subject to overflow. To eliminate
> this risk, this should use
>
> g_new0(mcd_server_info_st, num_servers)
>
> which will validate overflow & abort if hit.
>
> There are many more instances of this code pattern in the series
>
> $ git diff -r master | grep g_malloc | grep ' \* '
> + .tx = g_malloc(txlist->num_tx * sizeof(mcd_tx_st)),
> + server_info = g_malloc0(num_servers * sizeof(*server_info));
> + system_con_info = g_malloc0(num_systems * sizeof(*system_con_info));
> + device_con_info = g_malloc0(num_devices * sizeof(*device_con_info));
> + core_con_info = g_malloc0(num_cores * sizeof(*core_con_info));
> + memspaces = g_malloc0(num_mem_spaces * sizeof(*memspaces));
> + reg_groups = g_malloc0(num_reg_groups * sizeof(*reg_groups));
> + regs = g_malloc0(num_regs * sizeof(*regs));
> + ctrig_info = g_malloc0(num_ctrigs * sizeof(*ctrig_info));
> + trig_ids = g_malloc0(num_trigs * sizeof(*trig_ids));
>
>
> QEMU is a bit inconsistent, but we have a slight bias in favour
> of using g_new0, even for single struct allocations.
>
> IMHO being in the habit of always using g_new0 instead of g_malloc
> makes it less likely for people to inadvertantly introduce the
> multiplication overflow code pattern with g_malloc.
Oh, I didn't know that function, thanks for pointing it out! I agree,
it's the more elegant option.
