On 11/13/20 1:39 PM, Dr. David Alan Gilbert wrote:
> * Eric Blake (ebl...@redhat.com) wrote:
>> These cases require a bit more thought to review; in each case, the
>> code was appending to a list, but not with a FOOList **tail variable.
>>
>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>> ---
>
> <snip>
>
>> +++ b/monitor/hmp-cmds.c
>> @@ -1699,7 +1699,8 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
>> void hmp_sendkey(Monitor *mon, const QDict *qdict)
>> {
>> const char *keys = qdict_get_str(qdict, "keys");
>> - KeyValueList *keylist, *head = NULL, *tmp = NULL;
>> + KeyValue *v;
>> + KeyValueList *head = NULL, **tail = &head;
>> int has_hold_time = qdict_haskey(qdict, "hold-time");
>> int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
>> Error *err = NULL;
>> @@ -1716,16 +1717,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>> keyname_len = 4;
>> }
>>
>> - keylist = g_malloc0(sizeof(*keylist));
>> - keylist->value = g_malloc0(sizeof(*keylist->value));
>> -
>> - if (!head) {
>> - head = keylist;
>> - }
>> - if (tmp) {
>> - tmp->next = keylist;
>> - }
>> - tmp = keylist;
>> + v = g_malloc0(sizeof(*v));
>>
>> if (strstart(keys, "0x", NULL)) {
>> char *endp;
>> @@ -1734,16 +1726,17 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>> if (endp != keys + keyname_len) {
>> goto err_out;
>> }
>> - keylist->value->type = KEY_VALUE_KIND_NUMBER;
>> - keylist->value->u.number.data = value;
>> + v->type = KEY_VALUE_KIND_NUMBER;
>> + v->u.number.data = value;
>> } else {
>> int idx = index_from_key(keys, keyname_len);
>> if (idx == Q_KEY_CODE__MAX) {
>> goto err_out;
>> }
>> - keylist->value->type = KEY_VALUE_KIND_QCODE;
>> - keylist->value->u.qcode.data = idx;
>> + v->type = KEY_VALUE_KIND_QCODE;
>> + v->u.qcode.data = idx;
>> }
>> + QAPI_LIST_APPEND(tail, v);
>>
>> if (!*separator) {
>> break;
>
> Don't you need to arrange for 'v' to be free'd in the err_out case?
Good catch. Pre-patch, the allocation was appended to the list before
it was possible to reach 'goto err_out', but post-patch, the use of a
separate variable and delayed addition to the list matters. Will fix.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
