pulsemixer: Don't use g_atomic_int_(get|set) for accessing the mixer track flags
authorSebastian Dröge <sebastian.droege@collabora.co.uk>
Sun, 22 Feb 2009 17:08:59 +0000 (18:08 +0100)
committerSebastian Dröge <sebastian.droege@collabora.co.uk>
Sun, 22 Feb 2009 17:08:59 +0000 (18:08 +0100)
g_atomic_int_(get|set) only work on ints and the flags are
an enum (which on most architectures is stored as an int).

Also the way the flags were accessed atomically would still
leave a possible race condition and we don't do it in any
other mixer track implementation, let alone at any other
place where an integer could be changed from different
threads. Removing the g_atomic_int_(get|set) will only
introduce a new race condition on architectures where
integers could be half-written while reading them
which shouldn't be the case for any modern architecture
and if we really care about this we need to use
g_atomic_int_(get|set) at many other places too.

Apart from that g_atomic_int_(set|get) will result in
aliasing warnings if their argument is explicitely
casted to an int *. Fixes bug #571153.

ext/pulse/pulsemixerctrl.c

index 41b68cfcd2af930dbdd398f0727b4bfedc18505f..ae807c66099cb1a88f458b6837d103ac073a58c3 100644 (file)
@@ -93,10 +93,11 @@ gst_pulsemixer_ctrl_sink_info_cb (pa_context * context, const pa_sink_info * i,
   c->type = GST_PULSEMIXER_SINK;
 
   if (c->track) {
-    int i = g_atomic_int_get ((gint *) & c->track->flags);
+    GstMixerTrackFlags flags = c->track->flags;
 
-    i = (i & ~GST_MIXER_TRACK_MUTE) | (c->muted ? GST_MIXER_TRACK_MUTE : 0);
-    g_atomic_int_set ((gint *) & c->track->flags, i);
+    flags =
+        (flags & ~GST_MIXER_TRACK_MUTE) | (c->muted ? GST_MIXER_TRACK_MUTE : 0);
+    c->track->flags = flags;
   }
 
   c->operation_success = 1;
@@ -142,10 +143,11 @@ gst_pulsemixer_ctrl_source_info_cb (pa_context * context,
   c->type = GST_PULSEMIXER_SOURCE;
 
   if (c->track) {
-    int i = g_atomic_int_get ((gint *) & c->track->flags);
+    GstMixerTrackFlags flags = c->track->flags;
 
-    i = (i & ~GST_MIXER_TRACK_MUTE) | (c->muted ? GST_MIXER_TRACK_MUTE : 0);
-    g_atomic_int_set ((gint *) & c->track->flags, i);
+    flags =
+        (flags & ~GST_MIXER_TRACK_MUTE) | (c->muted ? GST_MIXER_TRACK_MUTE : 0);
+    c->track->flags = flags;
   }
 
   c->operation_success = 1;
@@ -572,10 +574,11 @@ gst_pulsemixer_ctrl_set_mute (GstPulseMixerCtrl * c, GstMixerTrack * track,
   c->update_mute = TRUE;
 
   if (c->track) {
-    int i = g_atomic_int_get ((gint *) & c->track->flags);
+    GstMixerTrackFlags flags = c->track->flags;
 
-    i = (i & ~GST_MIXER_TRACK_MUTE) | (c->muted ? GST_MIXER_TRACK_MUTE : 0);
-    g_atomic_int_set ((gint *) & c->track->flags, i);
+    flags =
+        (flags & ~GST_MIXER_TRACK_MUTE) | (c->muted ? GST_MIXER_TRACK_MUTE : 0);
+    c->track->flags = flags;
   }
 
   restart_time_event (c);