[Spice-devel] [PATCH] Avoid to access data before a NULL check

Frediano Ziglio fziglio at redhat.com
Wed May 25 10:31:13 UTC 2016


If you are testing for NULL data this means that variable could be
NULL so avoid to access before the check to make sure the check is hit.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/cursor-channel.c  |  5 +++--
 server/dcc-send.c        |  6 ++++--
 server/dcc.c             |  2 +-
 server/display-channel.c |  6 +++---
 server/sound.c           | 24 +++++++++++++-----------
 server/stream.c          |  3 ++-
 6 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index ab6864c..e91aac8 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -227,12 +227,13 @@ static void cursor_channel_client_on_disconnect(RedChannelClient *rcc)
 static void red_marshall_cursor_init(RedChannelClient *rcc, SpiceMarshaller *base_marshaller,
                                      RedPipeItem *pipe_item)
 {
+    spice_assert(rcc);
+
     CursorChannel *cursor_channel;
     CursorChannelClient *ccc = RCC_TO_CCC(rcc);
     SpiceMsgCursorInit msg;
     AddBufInfo info;
 
-    spice_assert(rcc);
     cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel, common.base);
 
     red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, NULL);
@@ -256,7 +257,7 @@ static void cursor_marshall(RedChannelClient *rcc,
     RedPipeItem *pipe_item = &cursor_pipe_item->base;
     RedCursorCmd *cmd;
 
-    spice_return_if_fail(cursor_channel);
+    spice_return_if_fail(rcc->channel);
 
     cmd = item->red_cursor;
     switch (cmd->type) {
diff --git a/server/dcc-send.c b/server/dcc-send.c
index 5171f9a..719eb35 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -1923,7 +1923,6 @@ static void red_marshall_image(RedChannelClient *rcc,
                                RedImageItem *item)
 {
     DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
-    DisplayChannel *display = DCC_TO_DC(dcc);
     SpiceImage red_image;
     SpiceBitmap bitmap;
     SpiceChunks *chunks;
@@ -1932,7 +1931,10 @@ static void red_marshall_image(RedChannelClient *rcc,
     SpiceMarshaller *src_bitmap_out, *mask_bitmap_out;
     SpiceMarshaller *bitmap_palette_out, *lzplt_palette_out;
 
-    spice_assert(rcc && display && item);
+    spice_assert(rcc && item);
+
+    DisplayChannel *display = DCC_TO_DC(dcc);
+    spice_assert(display);
 
     QXL_SET_IMAGE_ID(&red_image, QXL_IMAGE_GROUP_RED, display_channel_generate_uid(display));
     red_image.descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
diff --git a/server/dcc.c b/server/dcc.c
index ef43a3c..9cb296a 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -149,7 +149,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
     }
 
     display = DCC_TO_DC(dcc);
-    flags = is_primary_surface(DCC_TO_DC(dcc), surface_id) ? SPICE_SURFACE_FLAGS_PRIMARY : 0;
+    flags = is_primary_surface(display, surface_id) ? SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
     /* don't send redundant create surface commands to client */
     if (!dcc || display->common.during_target_migrate ||
diff --git a/server/display-channel.c b/server/display-channel.c
index 13be947..f5a4274 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -392,10 +392,10 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
             drawable_remove_from_pipes(drawable);
             current_remove_drawable(display, drawable);
         } else {
-            Container *container = CONTAINER(now);
-
             spice_assert(now->type == TREE_ITEM_TYPE_CONTAINER);
 
+            Container *container = CONTAINER(now);
+
             if ((ring_item = ring_get_head(&container->items))) {
                 now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
                 continue;
@@ -481,7 +481,7 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
             while (link) {
                 dcc = link->data;
                 dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
-                while (link && (!dpi || dcc != dpi->dcc)) {
+                while (link && (!dpi_ring_item || dcc != dpi->dcc)) {
                     dcc_prepend_drawable(dcc, drawable);
                     link = link->next;
                     if (link)
diff --git a/server/sound.c b/server/sound.c
index 8335101..eb05a6d 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1018,7 +1018,6 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_volume(SpicePlaybackInstance *
 {
     SpiceVolumeState *st = &sin->st->volume;
     SndChannel *channel = sin->st->worker.connection;
-    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
 
     st->volume_nchannels = nchannels;
     free(st->volume);
@@ -1026,6 +1025,7 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_volume(SpicePlaybackInstance *
 
     if (!channel || nchannels == 0)
         return;
+    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
 
     snd_playback_send_volume(playback_channel);
 }
@@ -1034,12 +1034,12 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance *si
 {
     SpiceVolumeState *st = &sin->st->volume;
     SndChannel *channel = sin->st->worker.connection;
-    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
 
     st->mute = mute;
 
     if (!channel)
         return;
+    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
 
     snd_playback_send_mute(playback_channel);
 }
@@ -1047,11 +1047,11 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance *si
 SPICE_GNUC_VISIBLE void spice_server_playback_start(SpicePlaybackInstance *sin)
 {
     SndChannel *channel = sin->st->worker.connection;
-    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
 
     sin->st->worker.active = 1;
     if (!channel)
         return;
+    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
     spice_assert(!playback_channel->base.active);
     reds_disable_mm_time(snd_channel_get_server(channel));
     playback_channel->base.active = TRUE;
@@ -1066,11 +1066,11 @@ SPICE_GNUC_VISIBLE void spice_server_playback_start(SpicePlaybackInstance *sin)
 SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance *sin)
 {
     SndChannel *channel = sin->st->worker.connection;
-    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
 
     sin->st->worker.active = 0;
     if (!channel)
         return;
+    PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
     spice_assert(playback_channel->base.active);
     reds_enable_mm_time(snd_channel_get_server(channel));
     playback_channel->base.active = FALSE;
@@ -1173,13 +1173,13 @@ static int snd_desired_audio_mode(int playback_compression, int frequency,
 
 static void on_new_playback_channel(SndWorker *worker)
 {
+    spice_assert(worker->connection);
+
     RedsState *reds = red_channel_get_server(worker->base_channel);
     PlaybackChannel *playback_channel =
         SPICE_CONTAINEROF(worker->connection, PlaybackChannel, base);
     SpicePlaybackState *st = SPICE_CONTAINEROF(worker, SpicePlaybackState, worker);
 
-    spice_assert(playback_channel);
-
     snd_set_command((SndChannel *)playback_channel, SND_PLAYBACK_MODE_MASK);
     if (playback_channel->base.active) {
         snd_set_command((SndChannel *)playback_channel, SND_PLAYBACK_CTRL_MASK);
@@ -1280,7 +1280,6 @@ SPICE_GNUC_VISIBLE void spice_server_record_set_volume(SpiceRecordInstance *sin,
 {
     SpiceVolumeState *st = &sin->st->volume;
     SndChannel *channel = sin->st->worker.connection;
-    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
 
     st->volume_nchannels = nchannels;
     free(st->volume);
@@ -1289,6 +1288,7 @@ SPICE_GNUC_VISIBLE void spice_server_record_set_volume(SpiceRecordInstance *sin,
     if (!channel || nchannels == 0)
         return;
 
+    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
     snd_record_send_volume(record_channel);
 }
 
@@ -1296,24 +1296,24 @@ SPICE_GNUC_VISIBLE void spice_server_record_set_mute(SpiceRecordInstance *sin, u
 {
     SpiceVolumeState *st = &sin->st->volume;
     SndChannel *channel = sin->st->worker.connection;
-    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
 
     st->mute = mute;
 
     if (!channel)
         return;
 
+    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
     snd_record_send_mute(record_channel);
 }
 
 SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
 {
     SndChannel *channel = sin->st->worker.connection;
-    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
 
     sin->st->worker.active = 1;
     if (!channel)
         return;
+    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
     spice_assert(!record_channel->base.active);
     record_channel->base.active = TRUE;
     record_channel->read_pos = record_channel->write_pos = 0;   //todo: improve by
@@ -1329,11 +1329,12 @@ SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
 SPICE_GNUC_VISIBLE void spice_server_record_stop(SpiceRecordInstance *sin)
 {
     SndChannel *channel = sin->st->worker.connection;
-    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
 
     sin->st->worker.active = 0;
     if (!channel)
         return;
+
+    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
     spice_assert(record_channel->base.active);
     record_channel->base.active = FALSE;
     if (record_channel->base.client_active) {
@@ -1348,13 +1349,14 @@ SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
                                                             uint32_t *samples, uint32_t bufsize)
 {
     SndChannel *channel = sin->st->worker.connection;
-    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
     uint32_t read_pos;
     uint32_t now;
     uint32_t len;
 
     if (!channel)
         return 0;
+
+    RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
     spice_assert(record_channel->base.active);
 
     if (record_channel->write_pos < RECORD_SAMPLES_SIZE / 2) {
diff --git a/server/stream.c b/server/stream.c
index 2aa13d1..6061229 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -789,9 +789,10 @@ void stream_agent_stop(StreamAgent *agent)
 
 static void red_upgrade_item_free(RedPipeItem *base)
 {
+    g_return_if_fail(base != NULL);
+
     RedUpgradeItem *item = SPICE_UPCAST(RedUpgradeItem, base);
 
-    g_return_if_fail(item != NULL);
     g_return_if_fail(item->base.refcount == 0);
 
     drawable_unref(item->drawable);
-- 
2.7.4



More information about the Spice-devel mailing list