Re: [Spice-devel] [RFC PATCH 2/2] Introduce some macro to simplify iteration on GList

2016-09-20 Thread Frediano Ziglio
> 
> On 09/16/2016 06:06 PM, Frediano Ziglio wrote:
> > Noting that coding by hand these loop introduced some regression
> > I'm trying to introduce back from macros.
> > Before trying something harder to make possible to bind the type of
> > the content I'm trying some simple macro as were before.
> > I added the type to avoid some blindly void* casts.
> > Also the GListIter is introduced to avoid the possibility to exchange
> > easily some parameters.
> >
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  server/red-common.h   | 18 ++
> >  server/reds-private.h |  3 +++
> >  server/reds.c | 66
> >  +--
> >  3 files changed, 53 insertions(+), 34 deletions(-)
> >
> > diff --git a/server/red-common.h b/server/red-common.h
> > index 7ab7e15..190fd9c 100644
> > --- a/server/red-common.h
> > +++ b/server/red-common.h
> > @@ -62,4 +62,22 @@ extern const SpiceCoreInterfaceInternal event_loop_core;
> >
> >  typedef struct RedsState RedsState;
> >
> > +typedef struct GListIter {
> > +GList *link;
> > +GList *next;
> > +} GListIter;
> > +
> > +#define GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, _dir) \
> > +for (_iter.link = _list; \
> > +(_data = (_type *) (_iter.link ? _iter.link->data : NULL), \
> > + _iter.next = (_iter.link ? _iter.link->_dir : NULL), \
> > + _iter.link) != NULL; \
> > + _iter.link = _iter.next)
> > +
> > +#define GLIST_FOREACH(_list, _iter, _type, _data) \
> > +GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, next)
> > +
> > +#define GLIST_FOREACH_REVERSED(_list, _iter, _type, _data) \
> > +GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, prev)
> > +
> >  #endif
> 
> 
> Hi Frediano,
> 
> Looks good.
> 
> GLIST_FOREACH_REVERSED is not used.
> 

It will.

> In the future it would be nice to set _type
> according to typeof _data, or compare _type with it.
> 

It's the reason of the cast. The compiler will give warning that with -Werror
become an error. Would be worth attaching type inside GList to GList field
but would require some hacks.

> Regards,
>  Uri.
> 

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


Re: [Spice-devel] [RFC PATCH 2/2] Introduce some macro to simplify iteration on GList

2016-09-19 Thread Uri Lublin

On 09/16/2016 06:06 PM, Frediano Ziglio wrote:

Noting that coding by hand these loop introduced some regression
I'm trying to introduce back from macros.
Before trying something harder to make possible to bind the type of
the content I'm trying some simple macro as were before.
I added the type to avoid some blindly void* casts.
Also the GListIter is introduced to avoid the possibility to exchange
easily some parameters.

Signed-off-by: Frediano Ziglio 
---
 server/red-common.h   | 18 ++
 server/reds-private.h |  3 +++
 server/reds.c | 66 +--
 3 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/server/red-common.h b/server/red-common.h
index 7ab7e15..190fd9c 100644
--- a/server/red-common.h
+++ b/server/red-common.h
@@ -62,4 +62,22 @@ extern const SpiceCoreInterfaceInternal event_loop_core;

 typedef struct RedsState RedsState;

+typedef struct GListIter {
+GList *link;
+GList *next;
+} GListIter;
+
+#define GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, _dir) \
+for (_iter.link = _list; \
+(_data = (_type *) (_iter.link ? _iter.link->data : NULL), \
+ _iter.next = (_iter.link ? _iter.link->_dir : NULL), \
+ _iter.link) != NULL; \
+ _iter.link = _iter.next)
+
+#define GLIST_FOREACH(_list, _iter, _type, _data) \
+GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, next)
+
+#define GLIST_FOREACH_REVERSED(_list, _iter, _type, _data) \
+GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, prev)
+
 #endif



Hi Frediano,

Looks good.

GLIST_FOREACH_REVERSED is not used.

In the future it would be nice to set _type
according to typeof _data, or compare _type with it.

Regards,
Uri.


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


[Spice-devel] [RFC PATCH 2/2] Introduce some macro to simplify iteration on GList

2016-09-16 Thread Frediano Ziglio
Noting that coding by hand these loop introduced some regression
I'm trying to introduce back from macros.
Before trying something harder to make possible to bind the type of
the content I'm trying some simple macro as were before.
I added the type to avoid some blindly void* casts.
Also the GListIter is introduced to avoid the possibility to exchange
easily some parameters.

Signed-off-by: Frediano Ziglio 
---
 server/red-common.h   | 18 ++
 server/reds-private.h |  3 +++
 server/reds.c | 66 +--
 3 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/server/red-common.h b/server/red-common.h
index 7ab7e15..190fd9c 100644
--- a/server/red-common.h
+++ b/server/red-common.h
@@ -62,4 +62,22 @@ extern const SpiceCoreInterfaceInternal event_loop_core;
 
 typedef struct RedsState RedsState;
 
+typedef struct GListIter {
+GList *link;
+GList *next;
+} GListIter;
+
+#define GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, _dir) \
+for (_iter.link = _list; \
+(_data = (_type *) (_iter.link ? _iter.link->data : NULL), \
+ _iter.next = (_iter.link ? _iter.link->_dir : NULL), \
+ _iter.link) != NULL; \
+ _iter.link = _iter.next)
+
+#define GLIST_FOREACH(_list, _iter, _type, _data) \
+GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, next)
+
+#define GLIST_FOREACH_REVERSED(_list, _iter, _type, _data) \
+GLIST_FOREACH_GENERIC(_list, _iter, _type, _data, prev)
+
 #endif
diff --git a/server/reds-private.h b/server/reds-private.h
index 29645bf..a044a57 100644
--- a/server/reds-private.h
+++ b/server/reds-private.h
@@ -156,4 +156,7 @@ struct RedsState {
 MainDispatcher *main_dispatcher;
 };
 
+#define FOREACH_QXL_INSTANCE(_reds, _iter, _qxl) \
+GLIST_FOREACH(_reds->qxl_instances, _iter, QXLInstance, _qxl)
+
 #endif
diff --git a/server/reds.c b/server/reds.c
index 8e9dc7b..1321797 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -688,15 +688,15 @@ int reds_get_mouse_mode(RedsState *reds)
 
 static void reds_set_mouse_mode(RedsState *reds, uint32_t mode)
 {
-GList *l;
+GListIter it;
+QXLInstance *qxl;
 
 if (reds->mouse_mode == mode) {
 return;
 }
 reds->mouse_mode = mode;
 
-for (l = reds->qxl_instances; l != NULL; l = l->next) {
-QXLInstance *qxl = (QXLInstance *)l->data;
+FOREACH_QXL_INSTANCE(reds, it, qxl) {
 red_qxl_set_mouse_mode(qxl, mode);
 }
 
@@ -1717,14 +1717,11 @@ static void reds_mig_target_client_free(RedsState 
*reds, RedsMigTargetClient *mi
 
 static void reds_mig_target_client_disconnect_all(RedsState *reds)
 {
-GList *l, *next;
+GListIter it;
+RedsMigTargetClient *mig_client;
 
-l = reds->mig_target_clients;
-while (l) {
-next = l->next;
-RedsMigTargetClient *mig_client = l->data;
+GLIST_FOREACH(reds->mig_target_clients, it, RedsMigTargetClient, 
mig_client) {
 reds_client_disconnect(reds, mig_client->client);
-l = next;
 }
 }
 
@@ -4315,13 +4312,14 @@ void reds_update_client_mouse_allowed(RedsState *reds)
 int allow_now = FALSE;
 int x_res = 0;
 int y_res = 0;
-GList *l;
 int num_active_workers = g_list_length(reds->qxl_instances);
 
 if (num_active_workers > 0) {
+GListIter it;
+QXLInstance *qxl;
+
 allow_now = TRUE;
-for (l = reds->qxl_instances; l != NULL && allow_now; l = l->next) {
-QXLInstance *qxl = l->data;
+FOREACH_QXL_INSTANCE(reds, it, qxl) {
 if (red_qxl_get_primary_active(qxl)) {
 allow_now = red_qxl_get_allow_client_mouse(qxl, &x_res, 
&y_res);
 break;
@@ -4342,15 +4340,14 @@ void reds_update_client_mouse_allowed(RedsState *reds)
 
 static gboolean reds_use_client_monitors_config(RedsState *reds)
 {
-GList *l;
+GListIter it;
+QXLInstance *qxl;
 
 if (reds->qxl_instances == NULL) {
 return FALSE;
 }
 
-for (l = reds->qxl_instances; l != NULL ; l = l->next) {
-QXLInstance *qxl = l->data;
-
+FOREACH_QXL_INSTANCE(reds, it, qxl) {
 if (!red_qxl_use_client_monitors_config(qxl))
 return FALSE;
 }
@@ -4359,10 +4356,10 @@ static gboolean 
reds_use_client_monitors_config(RedsState *reds)
 
 static void reds_client_monitors_config(RedsState *reds, VDAgentMonitorsConfig 
*monitors_config)
 {
-GList *l;
+GListIter it;
+QXLInstance *qxl;
 
-for (l = reds->qxl_instances; l != NULL; l = l->next) {
-QXLInstance *qxl = l->data;
+FOREACH_QXL_INSTANCE(reds, it, qxl) {
 if (!red_qxl_client_monitors_config(qxl, monitors_config)) {
 /* this is a normal condition, some qemu devices might not 
implement it */
 spice_debug("QXLInterface::client_monitors_config failed\n");
@@ -4385,10 +4382,10 @@ static int calc_compression_level(RedsState *reds)
 void reds_on_ic_change(RedsState *reds)
 {
 int compression_level = ca