[Spice-devel] [server PATCH 1/8] red_worker: use only RCC_FOREACH_SAFE

[Yonit Halperin]

Hi,

On 07/08/2013 06:32 AM, Uri Lublin wrote:
> RCC_FOREACH may be dangerous
>
> The following patches replace FOREACH loops with a SAFE version.
> Using unsafe loops may cause spice-server to abort (assert fails).
> Specifically a read/write fail in those loops, may cause the client
> to disconnect, removing the node currently iterated, which cause spice
> to abort in ring_next():
>   -- assertion `pos->next != NULL && pos->prev != NULL' failed
Now that red_channel was fixed to use RING_FOREACH_SAFE where needed, I 
think it would be nicer to reuse red_channel.c code, specifically, 
red_channel_pipes_new_add.

Regards,
Yonit.
> ---
>   server/red_worker.c |   11 ++---------
>   1 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 825bca0..a7f8d79 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -1113,13 +1113,6 @@ static inline uint64_t red_now(void);
>    *  given a channel, iterate over it's clients
>    */
>
> -#define RCC_FOREACH(link, rcc, channel) \
> -    for (link = ring_get_head(&(channel)->clients),\
> -         rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);\
> -            (link);                              \
> -            (link) = ring_next(&(channel)->clients, link),\
> -            rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link))
> -
>   #define RCC_FOREACH_SAFE(link, next, rcc, channel) \
>       for (link = ring_get_head(&(channel)->clients),                         \
>            rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link),     \
> @@ -1426,9 +1419,9 @@ static void red_push_surface_image(DisplayChannelClient *dcc, int surface_id);
>   static void red_pipes_add_verb(RedChannel *channel, uint16_t verb)
>   {
>       RedChannelClient *rcc;
> -    RingItem *link;
> +    RingItem *link, *next;
>
> -    RCC_FOREACH(link, rcc, channel) {
> +    RCC_FOREACH_SAFE(link, next, rcc, channel) {
>           red_pipe_add_verb(rcc, verb);
>       }
>   }
>