[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qapi: change QmpInputVisitor to QSLIST
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] qapi: change QmpInputVisitor to QSLIST |
Date: |
Thu, 07 Jul 2016 10:19:54 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 07/06/2016 06:43 AM, Paolo Bonzini wrote:
>> This saves a lot of memory compared to a statically-sized array.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> qapi/qmp-input-visitor.c | 53
>> ++++++++++++++++++++++++------------------------
>> 1 file changed, 26 insertions(+), 27 deletions(-)
>>
>
>> @@ -99,17 +100,10 @@ static const QListEntry *qmp_input_push(QmpInputVisitor
>> *qiv, QObject *obj,
>> Error **errp)
>> {
>> GHashTable *h;
>> - StackObject *tos = &qiv->stack[qiv->nb_stack];
>> + StackObject *tos = g_new0(StackObject, 1);
>>
>> assert(obj);
>> - if (qiv->nb_stack >= QIV_STACK_SIZE) {
>
> You should also delete QIV_STACK_SIZE as it is now unused.
Actually, you should either prove that untrusted input still cannot make
us allocated unbounded amounts of memory, or bring the limit right back.
>> @@ -127,9 +121,7 @@ static const QListEntry *qmp_input_push(QmpInputVisitor
>> *qiv, QObject *obj,
>> static void qmp_input_check_struct(Visitor *v, Error **errp)
>> {
>> QmpInputVisitor *qiv = to_qiv(v);
>> - StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
>> -
>> - assert(qiv->nb_stack > 0);
>> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
>
> Does QSLIST_FIRST() properly crash if the list is empty, or do we need
It returns null.
> to add an assert(tos) to replace the assertion on nb_stack being non-zero?
We do need to.
> Otherwise looking reasonable; looking forward to v2.
Re: [Qemu-devel] [PATCH] qapi: change QmpOutputVisitor to QSLIST, Eric Blake, 2016/07/06
Re: [Qemu-devel] [PATCH] qapi: change QmpOutputVisitor to QSLIST, Markus Armbruster, 2016/07/07