[Spice-devel] [spice-server PATCH 1/8] current_remove: rename internal variable 'container'

[Uri Lublin]

On 10/17/2016 12:45 PM, Frediano Ziglio wrote:
>>
>> It shadows the outer one.
>>
>> Signed-off-by: Uri Lublin <uril at redhat.com>
>> ---
>>
>> Possibly more meaningful names than container and container2
>> should be used
>>
>> ---
>>  server/display-channel.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/server/display-channel.c b/server/display-channel.c
>> index b9366b5..49650e1 100644
>> --- a/server/display-channel.c
>> +++ b/server/display-channel.c
>> @@ -359,16 +359,16 @@ static void current_remove(DisplayChannel *display,
>> TreeItem *item)
>>              drawable_remove_from_pipes(drawable);
>>              current_remove_drawable(display, drawable);
>>          } else {
>> -            Container *container = CONTAINER(now);
>> +            Container *container2 = CONTAINER(now);
>>
>>              spice_assert(now->type == TREE_ITEM_TYPE_CONTAINER);
>>
>> -            if ((ring_item = ring_get_head(&container->items))) {
>> +            if ((ring_item = ring_get_head(&container2->items))) {
>>                  now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
>>                  continue;
>>              }
>>              ring_item = now->siblings_link.prev;
>> -            container_free(container);
>> +            container_free(container2);
>>          }
>>          if (now == item) {
>>              return;
>
> Why not something like

Hi Frediano,

Your names are better.
Also possible container_of_now instead of now_container.

Thanks,
     Uri.

>
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 69edd35..0c6c1ef 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -350,7 +350,7 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
>
>      /* depth-first tree traversal, TODO: do a to tree_foreach()? */
>      for (;;) {
> -        Container *container = now->container;
> +        Container *now_container = now->container;
>          RingItem *ring_item;
>
>          if (now->type == TREE_ITEM_TYPE_DRAWABLE) {
> @@ -359,25 +359,25 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
>              drawable_remove_from_pipes(drawable);
>              current_remove_drawable(display, drawable);
>          } else {
> -            Container *container = CONTAINER(now);
> +            Container *now_as_container = CONTAINER(now);
>
>              spice_assert(now->type == TREE_ITEM_TYPE_CONTAINER);
>
> -            if ((ring_item = ring_get_head(&container->items))) {
> +            if ((ring_item = ring_get_head(&now_as_container->items))) {
>                  now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
>                  continue;
>              }
>              ring_item = now->siblings_link.prev;
> -            container_free(container);
> +            container_free(now_as_container);
>          }
>          if (now == item) {
>              return;
>          }
>
> -        if ((ring_item = ring_next(&container->items, ring_item))) {
> +        if ((ring_item = ring_next(&now_container->items, ring_item))) {
>              now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
>          } else {
> -            now = &container->base;
> +            now = &now_container->base;
>          }
>      }
>  }
>
>
> Frediano
>