[Spice-devel] [PATCH] fix for not clearing the cursor ring when the primary surface is destroyed

[Yonit Halperin]

On 06/23/2010 04:57 PM, Gerd Hoffmann wrote:
>> -static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size)
>> +static int red_process_cursor(RedWorker *worker, uint32_t
>> max_pipe_size, int *ring_is_empty)
>> {
>> QXLCommandExt ext_cmd;
>> int n = 0;
>>
>> + *ring_is_empty = FALSE;
>> while (!worker->cursor_channel ||
>> worker->cursor_channel->base.pipe_size<= max_pipe_size) {
>> if (!worker->qxl->st->qif->get_cursor_command(worker->qxl,&ext_cmd)) {
>> + *ring_is_empty = TRUE;
>
> Hmm, I think this extra parameter isn't needed.
> red_process_cursor() returns the number of commands processed.
>
>> @@ -9981,17 +9986,18 @@ static inline void
>> flush_display_commands(RedWorker *worker)
>> {
>
> Here you can do:
>
> (1) make sure there is room in the output pipe.
> (2) call red_process_commands().
> (2a) if return value is zero the ring is empty -> we are done.
> (2b) otherwise repeat (until timeout hits).
>
>> +static inline void flush_cursor_commands(RedWorker *worker)
>> +{
>
> I think instead of cut+paste you can change flush_display_commands to
> take a function pointer and a RedChannel as additional arguments, then
> call it with red_process_cursor + worker->cursor_channel or
> red_process_commands + worker->display_channel.
>
> cheers,
> Gerd
>
I don't think it simplifies. You need a callback for process_commands, 
and for channel_push, and you also need to know if the channel is still 
connected, which is prefromed by checking worker->display_channel and 
worker->cursor_channel; so you need to pass RedChannel**, and it becomes 
less readable.
Also testing if the channel is connected, and if there is room in the 
pipe before each call to process_commands is redundant.