[Spice-devel] [PATCH] worker: improve validation for update command

[Uri Lublin]

On 11/26/2015 03:15 PM, Pavel Grunt wrote:
> On Thu, 2015-11-26 at 12:26 +0000, Frediano Ziglio wrote:
>> If surface_id is not valid we should still release resource allocated
>> by red_get_update_cmd and from the guest.
>> This to reduce leaks in case of a race or another error in the guest
>> driver.
>> Also not issue a warning on invalid surface number to avoid filling
>> log space unconditionally.

Hi Frediano,

The patch looks good to me.

However I think the code should issue a warning.
Maybe we need to think of a mechanism to limit the number of such
warnings.

Thanks,
     Uri.

>>
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>> ---
>>   server/red_worker.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/server/red_worker.c b/server/red_worker.c
>> index 678b581..229ffe9 100644
>> --- a/server/red_worker.c
>> +++ b/server/red_worker.c
>> @@ -877,12 +877,10 @@ static int red_process_commands(RedWorker *worker,
>> uint32_t max_pipe_size, int *
>>                                      &update, ext_cmd.cmd.data)) {
>>                   break;
>>               }
>> -            if (!validate_surface(worker->display_channel,
>> update.surface_id)) {
>> -                spice_warning("Invalid surface in QXL_CMD_UPDATE");
>> -                break;
>> +            if (validate_surface(worker->display_channel, update.surface_id))
>> {
>> +                display_channel_draw(worker->display_channel, &update.area,
>> update.surface_id);
>> +                worker->qxl->st->qif->notify_update(worker->qxl,
>> update.update_id);
>>               }
>> -            display_channel_draw(worker->display_channel, &update.area,
>> update.surface_id);
>> -            worker->qxl->st->qif->notify_update(worker->qxl,
>> update.update_id);
>>               release_info_ext.group_id = ext_cmd.group_id;
>>               release_info_ext.info = update.release_info;
>>               worker->qxl->st->qif->release_resource(worker->qxl,
>> release_info_ext);
>
> Acked-by: Pavel Grunt <pgrunt at redhat.com>
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>