[Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[Ian Jackson]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 754 bytes --]

qemu's audio subdirectory contains a copy of BSD's sys-queue.h, which
defines a bunch of LIST_ macros.  This makes it difficult to build a
program made partly out of qemu and partly out of the Linux kernel[1],
since Linux has a different set of LIST_ macros.  It might also cause
trouble when mixing with BSD-derived code.

Under the circumstances it's probably best to rename the versions in
qemu.  The attached patch does this.

[1] You might well ask why anyone would want to do this.  In Xen we
are moving our emulation of IO devices from processes which run on the
host into a dedicated VM (one per actual VM) which we call a `stub
domain'.  This dedicated VM runs a very cut-down `operating system'
which uses some code from Linux.

Regards,
Ian.


[-- Attachment #2: rename LIST_... to QEMU_LIST_... --]
[-- Type: text/plain, Size: 10877 bytes --]

Index: audio/audio.c
===================================================================
RCS file: /sources/qemu/qemu/audio/audio.c,v
retrieving revision 1.20
diff -u -r1.20 audio.c
--- audio/audio.c	14 Jan 2008 04:24:28 -0000	1.20
+++ audio/audio.c	24 Jan 2008 11:40:33 -0000
@@ -793,8 +793,8 @@
             sw->rate = NULL;
         }
 
-        LIST_REMOVE (sw, entries);
-        LIST_REMOVE (sc, entries);
+        QEMU_LIST_REMOVE (sw, entries);
+        QEMU_LIST_REMOVE (sc, entries);
         qemu_free (sc);
         if (was_active) {
             /* We have removed soft voice from the capture:
@@ -837,8 +837,8 @@
             qemu_free (sw);
             return -1;
         }
-        LIST_INSERT_HEAD (&hw_cap->sw_head, sw, entries);
-        LIST_INSERT_HEAD (&hw->cap_head, sc, entries);
+        QEMU_LIST_INSERT_HEAD (&hw_cap->sw_head, sw, entries);
+        QEMU_LIST_INSERT_HEAD (&hw->cap_head, sc, entries);
 #ifdef DEBUG_CAPTURE
         asprintf (&sw->name, "for %p %d,%d,%d",
                   hw, sw->info.freq, sw->info.bits, sw->info.nchannels);
@@ -1706,12 +1706,12 @@
     card->audio = s;
     card->name = qemu_strdup (name);
     memset (&card->entries, 0, sizeof (card->entries));
-    LIST_INSERT_HEAD (&s->card_head, card, entries);
+    QEMU_LIST_INSERT_HEAD (&s->card_head, card, entries);
 }
 
 void AUD_remove_card (QEMUSoundCard *card)
 {
-    LIST_REMOVE (card, entries);
+    QEMU_LIST_REMOVE (card, entries);
     card->audio = NULL;
     qemu_free (card->name);
 }
@@ -1723,9 +1723,9 @@
     const char *drvname;
     AudioState *s = &glob_audio_state;
 
-    LIST_INIT (&s->hw_head_out);
-    LIST_INIT (&s->hw_head_in);
-    LIST_INIT (&s->cap_head);
+    QEMU_LIST_INIT (&s->hw_head_out);
+    QEMU_LIST_INIT (&s->hw_head_in);
+    QEMU_LIST_INIT (&s->cap_head);
     atexit (audio_atexit);
 
     s->ts = qemu_new_timer (vm_clock, audio_timer, s);
@@ -1817,7 +1817,7 @@
         return NULL;
     }
 
-    LIST_INIT (&s->card_head);
+    QEMU_LIST_INIT (&s->card_head);
     register_savevm ("audio", 0, 1, audio_save, audio_load, s);
     qemu_mod_timer (s->ts, qemu_get_clock (vm_clock) + conf.period.ticks);
     return s;
@@ -1855,7 +1855,7 @@
 
     cap = audio_pcm_capture_find_specific (s, as);
     if (cap) {
-        LIST_INSERT_HEAD (&cap->cb_head, cb, entries);
+        QEMU_LIST_INSERT_HEAD (&cap->cb_head, cb, entries);
         return cap;
     }
     else {
@@ -1870,8 +1870,8 @@
         }
 
         hw = &cap->hw;
-        LIST_INIT (&hw->sw_head);
-        LIST_INIT (&cap->cb_head);
+        QEMU_LIST_INIT (&hw->sw_head);
+        QEMU_LIST_INIT (&cap->cb_head);
 
         /* XXX find a more elegant way */
         hw->samples = 4096 * 4;
@@ -1899,8 +1899,8 @@
             [hw->info.swap_endianness]
             [audio_bits_to_index (hw->info.bits)];
 
-        LIST_INSERT_HEAD (&s->cap_head, cap, entries);
-        LIST_INSERT_HEAD (&cap->cb_head, cb, entries);
+        QEMU_LIST_INSERT_HEAD (&s->cap_head, cap, entries);
+        QEMU_LIST_INSERT_HEAD (&cap->cb_head, cb, entries);
 
         hw = NULL;
         while ((hw = audio_pcm_hw_find_any_out (s, hw))) {
@@ -1926,7 +1926,7 @@
     for (cb = cap->cb_head.lh_first; cb; cb = cb->entries.le_next) {
         if (cb->opaque == cb_opaque) {
             cb->ops.destroy (cb_opaque);
-            LIST_REMOVE (cb, entries);
+            QEMU_LIST_REMOVE (cb, entries);
             qemu_free (cb);
 
             if (!cap->cb_head.lh_first) {
@@ -1943,12 +1943,12 @@
                         st_rate_stop (sw->rate);
                         sw->rate = NULL;
                     }
-                    LIST_REMOVE (sw, entries);
-                    LIST_REMOVE (sc, entries);
+                    QEMU_LIST_REMOVE (sw, entries);
+                    QEMU_LIST_REMOVE (sc, entries);
                     qemu_free (sc);
                     sw = sw1;
                 }
-                LIST_REMOVE (cap, entries);
+                QEMU_LIST_REMOVE (cap, entries);
                 qemu_free (cap);
             }
             return;
Index: audio/audio.h
===================================================================
RCS file: /sources/qemu/qemu/audio/audio.h,v
retrieving revision 1.11
diff -u -r1.11 audio.h
--- audio/audio.h	17 Nov 2007 17:14:39 -0000	1.11
+++ audio/audio.h	24 Jan 2008 11:40:33 -0000
@@ -70,7 +70,7 @@
 typedef struct CaptureState {
     void *opaque;
     struct capture_ops ops;
-    LIST_ENTRY (CaptureState) entries;
+    QEMU_LIST_ENTRY (CaptureState) entries;
 } CaptureState;
 
 typedef struct SWVoiceOut SWVoiceOut;
@@ -80,7 +80,7 @@
 typedef struct QEMUSoundCard {
     AudioState *audio;
     char *name;
-    LIST_ENTRY (QEMUSoundCard) entries;
+    QEMU_LIST_ENTRY (QEMUSoundCard) entries;
 } QEMUSoundCard;
 
 typedef struct QEMUAudioTimeStamp {
Index: audio/audio_int.h
===================================================================
RCS file: /sources/qemu/qemu/audio/audio_int.h,v
retrieving revision 1.12
diff -u -r1.12 audio_int.h
--- audio/audio_int.h	14 Jan 2008 04:24:28 -0000	1.12
+++ audio/audio_int.h	24 Jan 2008 11:40:33 -0000
@@ -79,10 +79,10 @@
     st_sample_t *mix_buf;
 
     int samples;
-    LIST_HEAD (sw_out_listhead, SWVoiceOut) sw_head;
-    LIST_HEAD (sw_cap_listhead, SWVoiceCap) cap_head;
+    QEMU_LIST_HEAD (sw_out_listhead, SWVoiceOut) sw_head;
+    QEMU_LIST_HEAD (sw_cap_listhead, SWVoiceCap) cap_head;
     struct audio_pcm_ops *pcm_ops;
-    LIST_ENTRY (HWVoiceOut) entries;
+    QEMU_LIST_ENTRY (HWVoiceOut) entries;
 } HWVoiceOut;
 
 typedef struct HWVoiceIn {
@@ -98,9 +98,9 @@
     st_sample_t *conv_buf;
 
     int samples;
-    LIST_HEAD (sw_in_listhead, SWVoiceIn) sw_head;
+    QEMU_LIST_HEAD (sw_in_listhead, SWVoiceIn) sw_head;
     struct audio_pcm_ops *pcm_ops;
-    LIST_ENTRY (HWVoiceIn) entries;
+    QEMU_LIST_ENTRY (HWVoiceIn) entries;
 } HWVoiceIn;
 
 struct SWVoiceOut {
@@ -116,7 +116,7 @@
     char *name;
     volume_t vol;
     struct audio_callback callback;
-    LIST_ENTRY (SWVoiceOut) entries;
+    QEMU_LIST_ENTRY (SWVoiceOut) entries;
 };
 
 struct SWVoiceIn {
@@ -131,7 +131,7 @@
     char *name;
     volume_t vol;
     struct audio_callback callback;
-    LIST_ENTRY (SWVoiceIn) entries;
+    QEMU_LIST_ENTRY (SWVoiceIn) entries;
 };
 
 struct audio_driver {
@@ -165,20 +165,20 @@
 struct capture_callback {
     struct audio_capture_ops ops;
     void *opaque;
-    LIST_ENTRY (capture_callback) entries;
+    QEMU_LIST_ENTRY (capture_callback) entries;
 };
 
 struct CaptureVoiceOut {
     HWVoiceOut hw;
     void *buf;
-    LIST_HEAD (cb_listhead, capture_callback) cb_head;
-    LIST_ENTRY (CaptureVoiceOut) entries;
+    QEMU_LIST_HEAD (cb_listhead, capture_callback) cb_head;
+    QEMU_LIST_ENTRY (CaptureVoiceOut) entries;
 };
 
 struct SWVoiceCap {
     SWVoiceOut sw;
     CaptureVoiceOut *cap;
-    LIST_ENTRY (SWVoiceCap) entries;
+    QEMU_LIST_ENTRY (SWVoiceCap) entries;
 };
 
 struct AudioState {
@@ -186,10 +186,10 @@
     void *drv_opaque;
 
     QEMUTimer *ts;
-    LIST_HEAD (card_listhead, QEMUSoundCard) card_head;
-    LIST_HEAD (hw_in_listhead, HWVoiceIn) hw_head_in;
-    LIST_HEAD (hw_out_listhead, HWVoiceOut) hw_head_out;
-    LIST_HEAD (cap_listhead, CaptureVoiceOut) cap_head;
+    QEMU_LIST_HEAD (card_listhead, QEMUSoundCard) card_head;
+    QEMU_LIST_HEAD (hw_in_listhead, HWVoiceIn) hw_head_in;
+    QEMU_LIST_HEAD (hw_out_listhead, HWVoiceOut) hw_head_out;
+    QEMU_LIST_HEAD (cap_listhead, CaptureVoiceOut) cap_head;
     int nb_hw_voices_out;
     int nb_hw_voices_in;
 };
Index: audio/audio_template.h
===================================================================
RCS file: /sources/qemu/qemu/audio/audio_template.h,v
retrieving revision 1.9
diff -u -r1.9 audio_template.h
--- audio/audio_template.h	17 Feb 2007 22:19:29 -0000	1.9
+++ audio/audio_template.h	24 Jan 2008 11:40:33 -0000
@@ -186,12 +186,12 @@
 
 static void glue (audio_pcm_hw_add_sw_, TYPE) (HW *hw, SW *sw)
 {
-    LIST_INSERT_HEAD (&hw->sw_head, sw, entries);
+    QEMU_LIST_INSERT_HEAD (&hw->sw_head, sw, entries);
 }
 
 static void glue (audio_pcm_hw_del_sw_, TYPE) (SW *sw)
 {
-    LIST_REMOVE (sw, entries);
+    QEMU_LIST_REMOVE (sw, entries);
 }
 
 static void glue (audio_pcm_hw_gc_, TYPE) (AudioState *s, HW **hwp)
@@ -202,7 +202,7 @@
 #ifdef DAC
         audio_detach_capture (hw);
 #endif
-        LIST_REMOVE (hw, entries);
+        QEMU_LIST_REMOVE (hw, entries);
         glue (s->nb_hw_voices_, TYPE) += 1;
         glue (audio_pcm_hw_free_resources_ ,TYPE) (hw);
         glue (hw->pcm_ops->fini_, TYPE) (hw);
@@ -267,9 +267,9 @@
     }
 
     hw->pcm_ops = drv->pcm_ops;
-    LIST_INIT (&hw->sw_head);
+    QEMU_LIST_INIT (&hw->sw_head);
 #ifdef DAC
-    LIST_INIT (&hw->cap_head);
+    QEMU_LIST_INIT (&hw->cap_head);
 #endif
     if (glue (hw->pcm_ops->init_, TYPE) (hw, as)) {
         goto err0;
@@ -294,7 +294,7 @@
         goto err1;
     }
 
-    LIST_INSERT_HEAD (&s->glue (hw_head_, TYPE), hw, entries);
+    QEMU_LIST_INSERT_HEAD (&s->glue (hw_head_, TYPE), hw, entries);
     glue (s->nb_hw_voices_, TYPE) -= 1;
 #ifdef DAC
     audio_attach_capture (s, hw);
Index: audio/sys-queue.h
===================================================================
RCS file: /sources/qemu/qemu/audio/sys-queue.h,v
retrieving revision 1.1
diff -u -r1.1 sys-queue.h
--- audio/sys-queue.h	30 Oct 2005 18:58:22 -0000	1.1
+++ audio/sys-queue.h	24 Jan 2008 11:40:35 -0000
@@ -64,12 +64,12 @@
 /*
  * List definitions.
  */
-#define LIST_HEAD(name, type)						\
+#define QEMU_LIST_HEAD(name, type)					\
 struct name {								\
 	struct type *lh_first;	/* first element */			\
 }
 
-#define LIST_ENTRY(type)						\
+#define QEMU_LIST_ENTRY(type)						\
 struct {								\
 	struct type *le_next;	/* next element */			\
 	struct type **le_prev;	/* address of previous next element */	\
@@ -78,11 +78,11 @@
 /*
  * List functions.
  */
-#define	LIST_INIT(head) {						\
+#define	QEMU_LIST_INIT(head) {						\
 	(head)->lh_first = NULL;					\
 }
 
-#define LIST_INSERT_AFTER(listelm, elm, field) {			\
+#define QEMU_LIST_INSERT_AFTER(listelm, elm, field) {			\
 	if (((elm)->field.le_next = (listelm)->field.le_next) != NULL)	\
 		(listelm)->field.le_next->field.le_prev =		\
 		    &(elm)->field.le_next;				\
@@ -90,14 +90,14 @@
 	(elm)->field.le_prev = &(listelm)->field.le_next;		\
 }
 
-#define LIST_INSERT_HEAD(head, elm, field) {				\
+#define QEMU_LIST_INSERT_HEAD(head, elm, field) {			\
 	if (((elm)->field.le_next = (head)->lh_first) != NULL)		\
 		(head)->lh_first->field.le_prev = &(elm)->field.le_next;\
 	(head)->lh_first = (elm);					\
 	(elm)->field.le_prev = &(head)->lh_first;			\
 }
 
-#define LIST_REMOVE(elm, field) {					\
+#define QEMU_LIST_REMOVE(elm, field) {					\
 	if ((elm)->field.le_next != NULL)				\
 		(elm)->field.le_next->field.le_prev = 			\
 		    (elm)->field.le_prev;				\

[Qemu-devel] Re: [PATCH] avoid name clashes due to LIST_* macros

[Ian Jackson]

iwj writes ("[PATCH] avoid name clashes due to LIST_* macros"):
> qemu's audio subdirectory contains a copy of BSD's sys-queue.h, which
> defines a bunch of LIST_ macros.  This makes it difficult to build a
> program made partly out of qemu and partly out of the Linux kernel[1],
> since Linux has a different set of LIST_ macros.  It might also cause
> trouble when mixing with BSD-derived code.
> 
> Under the circumstances it's probably best to rename the versions in
> qemu.  The attached patch does this.

Was there something wrong with my patch ?  I don't seem to have seen
any replies to it.

Ian.

Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[Anthony Liguori]

Ian Jackson wrote:
> qemu's audio subdirectory contains a copy of BSD's sys-queue.h, which
> defines a bunch of LIST_ macros.  This makes it difficult to build a
> program made partly out of qemu and partly out of the Linux kernel[1],
> since Linux has a different set of LIST_ macros.  It might also cause
> trouble when mixing with BSD-derived code.
>   

That doesn't seem like a very good justification.  If you're mixing QEMU 
code with other code, it's easier for you to maintain these merge 
conflict fixes as normal QEMU developers would have no idea what it 
wasn't okay to just use LIST_xxx

Regards,

Anthony Liguori

> Under the circumstances it's probably best to rename the versions in
> qemu.  The attached patch does this.
>
> [1] You might well ask why anyone would want to do this.  In Xen we
> are moving our emulation of IO devices from processes which run on the
> host into a dedicated VM (one per actual VM) which we call a `stub
> domain'.  This dedicated VM runs a very cut-down `operating system'
> which uses some code from Linux.
>
> Regards,
> Ian.
>
>   

Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[Ian Jackson]

Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros"):
> Ian Jackson wrote:
> > qemu's audio subdirectory contains a copy of BSD's sys-queue.h, which
> > defines a bunch of LIST_ macros.  This makes it difficult to build a
> > program made partly out of qemu and partly out of the Linux kernel[1],
> > since Linux has a different set of LIST_ macros.  It might also cause
> > trouble when mixing with BSD-derived code.
> 
> That doesn't seem like a very good justification.  If you're mixing QEMU 
> code with other code,

Well, surely with something like qemu one might expect to mix the code
with other things ?

Or is it the view of qemu upstream that qemu is not supposed to be
portable to such embedded environments derived from Linux ?  (Or BSD
for that matter - since these LIST_* names came from BSD originally.)

>   it's easier for you to maintain these merge 
> conflict fixes as normal QEMU developers would have no idea what it 
> wasn't okay to just use LIST_xxx

This merge conflict is impractical to fix every time.  Surely it is
better just to fix it once in one place ?  Normal qemu developers will
know that they can use QEMU_LIST_... because that's what will be
defined in the qemu tree.

qemu developers who copy-and-paste code from other projects should IMO
think more carefully about name clashes.

Ian.

Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[Johannes Schindelin]

Hi,

On Thu, 7 Feb 2008, Ian Jackson wrote:

> Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros"):
> > Ian Jackson wrote:
> >
> > > qemu's audio subdirectory contains a copy of BSD's sys-queue.h, 
> > > which defines a bunch of LIST_ macros.  This makes it difficult to 
> > > build a program made partly out of qemu and partly out of the Linux 
> > > kernel[1], since Linux has a different set of LIST_ macros.  It 
> > > might also cause trouble when mixing with BSD-derived code.
> > 
> > That doesn't seem like a very good justification.  If you're mixing 
> > QEMU code with other code,
> 
> Well, surely with something like qemu one might expect to mix the code 
> with other things ?

Read what you wrote.  By that reasoning you cannot use _any_ name in qemu, 
because qemu should bend over to be mixable with other code.

Now, if you seriously want to integrate the audiosubsystem with something 
in the Linux kernel, you might rename the macros.  But unless you 
contribute that code, and people like it, I guess you will not stand much 
of a chance to get the renaming patch into QEmu.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[Ian Jackson]

Johannes Schindelin writes ("Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros"):
> Read what you wrote.  By that reasoning you cannot use _any_ name in qemu, 
> because qemu should bend over to be mixable with other code.

No, not at all.  Just that when a clash happens something sensible
should be done: one or the other should be renamed.

> Now, if you seriously want to integrate the audiosubsystem with something 
> in the Linux kernel, you might rename the macros.  But unless you 
> contribute that code, and people like it, I guess you will not stand much 
> of a chance to get the renaming patch into QEmu.

This problem is not theoretical.  I wouldn't be arguing about it
otherwise.  Without this change, our minios qemu does not build.

Ian.

Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros

[malc]

On Thu, 7 Feb 2008, Ian Jackson wrote:

> Johannes Schindelin writes ("Re: [Qemu-devel] [PATCH] avoid name clashes due to LIST_* macros"):
>> Read what you wrote.  By that reasoning you cannot use _any_ name in qemu,
>> because qemu should bend over to be mixable with other code.
>
> No, not at all.  Just that when a clash happens something sensible
> should be done: one or the other should be renamed.

A bit of history: audio/sys-qemu.h only exists because when i was working
on Windows side of things plain <sys/queue.h> wasn't available for Mingw
(maybe things changed since then) so i took the liberty of copying the
header from my linux system and using it.

It would have been interesting if <sys/queue.h> was part of Mingw because
i doubt that you would have pushed for changing the system supplied
header.

Anyway that was just to put things into perspective.

>> Now, if you seriously want to integrate the audiosubsystem with something
>> in the Linux kernel, you might rename the macros.  But unless you
>> contribute that code, and people like it, I guess you will not stand much
>> of a chance to get the renaming patch into QEmu.
>
> This problem is not theoretical.  I wouldn't be arguing about it
> otherwise.  Without this change, our minios qemu does not build.

-- 
mailto:av1474@comtv.ru