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

2016-10-26 Thread Uri Lublin 

On 10/17/2016 12:45 PM, Frediano Ziglio wrote:


It shadows the outer one.

Signed-off-by: Uri Lublin 
---

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(>items))) {
+if ((ring_item = ring_get_head(>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(>items))) {
+if ((ring_item = ring_get_head(_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(>items, ring_item))) {
+if ((ring_item = ring_next(_container->items, ring_item))) {
 now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
 } else {
-now = >base;
+now = _container->base;
 }
 }
 }


Frediano



___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2016-10-17 Thread Frediano Ziglio 
> 
> It shadows the outer one.
> 
> Signed-off-by: Uri Lublin 
> ---
> 
> 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(>items))) {
> +if ((ring_item = ring_get_head(>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


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(>items))) {
+if ((ring_item = ring_get_head(_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(>items, ring_item))) {
+if ((ring_item = ring_next(_container->items, ring_item))) {
 now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
 } else {
-now = >base;
+now = _container->base;
 }
 }
 }


Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2016-10-16 Thread Uri Lublin 
It shadows the outer one.

Signed-off-by: Uri Lublin 
---

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(>items))) {
+if ((ring_item = ring_get_head(>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;
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel