vss: Propagate return value of afh_get_chunk().

With dynamic chunks, afh_get_chunk() may fail. Currently
vss_get_chunk() prints an error message in this case and returns
the null pointer. However, some callers of vss_get_chunk() happily
dereference the returned pointer without checking for NULL.

This patch modifies vss_get_chunk() to return int and teaches all
callers to check the return value. For the udp and dccp transport,
we disable the fec client temporarily in the error case while for
the http transport we log the error (but otherwise ignore it), and
try to continue with the next chunk.

This flaw was noticed by the clang static analyzer.

diff --git a/vss.c b/vss.c
index cf51f43f32608c72f56d1af0b8ad72f25c21161f..6aac9d59b5107e1a1c4118a961d54f4d500686e7 100644 (file)
--- a/vss.c
+++ b/vss.c
@@ -345,7 +345,7 @@ static int initialize_fec_client(struct fec_client *fc, struct vss_task *vsst)
        return 1;
 }
 
-static void vss_get_chunk(int chunk_num, struct vss_task *vsst,
+static int vss_get_chunk(int chunk_num, struct vss_task *vsst,
                char **buf, size_t *sz)
 {
        int ret;
@@ -362,31 +362,32 @@ static void vss_get_chunk(int chunk_num, struct vss_task *vsst,
                assert(vsst->header_buf);
                *buf = vsst->header_buf; /* stripped header */
                *sz = vsst->header_len;
-               return;
+               return 0;
        }
        ret = afh_get_chunk(chunk_num, &mmd->afd.afhi,
                mmd->afd.audio_format_id, vsst->map, vsst->mapsize,
                (const char **)buf, sz, &vsst->afh_context);
        if (ret < 0) {
-               PARA_WARNING_LOG("could not get chunk %d: %s\n",
-                       chunk_num, para_strerror(-ret));
                *buf = NULL;
                *sz = 0;
        }
+       return ret;
 }
 
-static void compute_group_size(struct vss_task *vsst, struct fec_group *g,
+static int compute_group_size(struct vss_task *vsst, struct fec_group *g,
                int max_bytes)
 {
        char *buf;
        size_t len;
-       int i, max_chunks = PARA_MAX(1LU, 150 / tv2ms(vss_chunk_time()));
+       int ret, i, max_chunks = PARA_MAX(1LU, 150 / tv2ms(vss_chunk_time()));
 
        if (g->first_chunk == 0) {
                g->num_chunks = 1;
-               vss_get_chunk(0, vsst, &buf, &len);
+               ret = vss_get_chunk(0, vsst, &buf, &len);
+               if (ret < 0)
+                        return ret;
                g->bytes = len;
-               return;
+               return 0;
        }
 
        g->num_chunks = 0;
@@ -404,7 +405,9 @@ static void compute_group_size(struct vss_task *vsst, struct fec_group *g,
                        break;
                if (chunk_num >= mmd->afd.afhi.chunks_total) /* eof */
                        break;
-               vss_get_chunk(chunk_num, vsst, &buf, &len);
+               ret = vss_get_chunk(chunk_num, vsst, &buf, &len);
+               if (ret < 0)
+                        return ret;
                if (g->bytes + len > max_bytes)
                        break;
                /* Include this chunk */
@@ -412,6 +415,7 @@ static void compute_group_size(struct vss_task *vsst, struct fec_group *g,
                g->num_chunks++;
        }
        assert(g->num_chunks);
+       return 1;
 }
 
 /*
@@ -473,7 +477,9 @@ static int compute_slice_size(struct fec_client *fc, struct vss_task *vsst)
        if (!need_audio_header(fc, vsst)) {
                max_group_bytes = k * max_slice_bytes;
                g->num_header_slices = 0;
-               compute_group_size(vsst, g, max_group_bytes);
+               ret = compute_group_size(vsst, g, max_group_bytes);
+               if (ret < 0)
+                       return ret;
                g->slice_bytes = DIV_ROUND_UP(g->bytes, k);
                if (g->slice_bytes == 0)
                        g->slice_bytes = 1;
@@ -489,7 +495,9 @@ static int compute_slice_size(struct fec_client *fc, struct vss_task *vsst)
        h = vsst->header_len;
        max_group_bytes = (k - num_slices(h, max_slice_bytes, n - k))
                * max_slice_bytes;
-       compute_group_size(vsst, g, max_group_bytes);
+       ret = compute_group_size(vsst, g, max_group_bytes);
+       if (ret < 0)
+               return ret;
        d = g->bytes;
        if (d == 0) {
                g->slice_bytes = DIV_ROUND_UP(h, k);
@@ -596,7 +604,9 @@ static int setup_next_fec_group(struct fec_client *fc, struct vss_task *vsst)
         * Setup data slices. Note that for ogg streams chunk 0 points to a
         * buffer on the heap rather than to the mapped audio file.
         */
-       vss_get_chunk(g->first_chunk, vsst, &buf, &len);
+       ret = vss_get_chunk(g->first_chunk, vsst, &buf, &len);
+       if (ret < 0)
+               return ret;
        for (p = buf; i < g->num_header_slices + data_slices; i++) {
                if (p + g->slice_bytes > buf + g->bytes) {
                        /*
@@ -1020,7 +1030,7 @@ err:
  */
 static void vss_send(struct vss_task *vsst)
 {
-       int i;
+       int i, ret;
        bool fec_active = false;
        struct timeval due;
        struct fec_client *fc, *tmp_fc;
@@ -1063,16 +1073,22 @@ static void vss_send(struct vss_task *vsst)
                        mmd->events++;
                        set_mmd_offset();
                }
-               vss_get_chunk(mmd->current_chunk, vsst, &buf, &len);
-               for (i = 0; senders[i].name; i++) {
+               ret = vss_get_chunk(mmd->current_chunk, vsst, &buf, &len);
+               if (ret < 0) {
+                       PARA_ERROR_LOG("could not get chunk %lu: %s\n",
+                               mmd->current_chunk, para_strerror(-ret));
+               } else {
                        /*
                         * We call ->send() even if len is zero because senders
                         * might have data queued which can be sent now.
                         */
-                       if (!senders[i].send)
-                               continue;
-                       senders[i].send(mmd->current_chunk, mmd->chunks_sent,
-                               buf, len, vsst->header_buf, vsst->header_len);
+                       for (i = 0; senders[i].name; i++) {
+                               if (!senders[i].send)
+                                       continue;
+                               senders[i].send(mmd->current_chunk,
+                                       mmd->chunks_sent, buf, len,
+                                       vsst->header_buf, vsst->header_len);
+                       }
                }
                mmd->chunks_sent++;
                mmd->current_chunk++;