[pulseaudio-discuss] [PATCH 10/23] sink-input, source-output: Use "goto fail" for error handling in new()

Tanu Kaskinen tanu.kaskinen at linux.intel.com
Wed Nov 20 01:26:02 PST 2013


In a later patch the sink input/source output structure will be
allocated in the very beginning of the function. This patch prepares
for that by making sure that the allocated structure will always be
properly cleaned up in case of a failure.
---
 src/pulsecore/sink-input.c    | 102 ++++++++++++++++++++++++++++++---------
 src/pulsecore/source-output.c | 110 +++++++++++++++++++++++++++++++++---------
 2 files changed, 165 insertions(+), 47 deletions(-)

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 30acf25..1f88bfc 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -321,16 +321,27 @@ int pa_sink_input_new(
         pa_sink_input_new_data_set_formats(data, tmp);
     }
 
-    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SINK_INPUT_NEW], data)) < 0)
-        return r;
+    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SINK_INPUT_NEW], data)) < 0) {
+        ret = r;
+        goto fail;
+    }
 
-    pa_return_val_if_fail(!data->driver || pa_utf8_valid(data->driver), -PA_ERR_INVALID);
+    if (data->driver && !pa_utf8_valid(data->driver)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->sink) {
         pa_sink *sink = pa_namereg_get(core, NULL, PA_NAMEREG_SINK);
-        pa_return_val_if_fail(sink, -PA_ERR_NOENTITY);
+
+        if (!sink) {
+            ret = -PA_ERR_NOENTITY;
+            goto fail;
+        }
+
         pa_sink_input_new_data_set_sink(data, sink, false);
     }
+
     /* Routing's done, we have a sink. Now let's fix the format and set up the
      * sample spec */
 
@@ -349,27 +360,45 @@ int pa_sink_input_new(
         PA_IDXSET_FOREACH(format, data->req_formats, idx)
             pa_log_info(" -- %s", pa_format_info_snprint(fmt, sizeof(fmt), format));
 
-        return -PA_ERR_NOTSUPPORTED;
+        ret = -PA_ERR_NOTSUPPORTED;
+        goto fail;
     }
 
     /* Now populate the sample spec and format according to the final
      * format that we've negotiated */
-    pa_return_val_if_fail(pa_format_info_to_sample_spec(data->format, &ss, &map) == 0, -PA_ERR_INVALID);
+
+    if (pa_format_info_to_sample_spec(data->format, &ss, &map) < 0) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
+
     pa_sink_input_new_data_set_sample_spec(data, &ss);
     if (pa_format_info_is_pcm(data->format) && pa_channel_map_valid(&map))
         pa_sink_input_new_data_set_channel_map(data, &map);
 
-    pa_return_val_if_fail(PA_SINK_IS_LINKED(pa_sink_get_state(data->sink)), -PA_ERR_BADSTATE);
-    pa_return_val_if_fail(!data->sync_base || (data->sync_base->sink == data->sink && pa_sink_input_get_state(data->sync_base) == PA_SINK_INPUT_CORKED), -PA_ERR_INVALID);
+    if (!PA_SINK_IS_LINKED(pa_sink_get_state(data->sink))) {
+        ret = -PA_ERR_BADSTATE;
+        goto fail;
+    }
+
+    if (data->sync_base && !(data->sync_base->sink == data->sink && pa_sink_input_get_state(data->sync_base) == PA_SINK_INPUT_CORKED)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     r = check_passthrough_connection(pa_sink_input_new_data_is_passthrough(data), data->sink);
-    if (r != PA_OK)
-        return r;
+    if (r != PA_OK) {
+        ret = r;
+        goto fail;
+    }
 
     if (!data->sample_spec_is_set)
         data->sample_spec = data->sink->sample_spec;
 
-    pa_return_val_if_fail(pa_sample_spec_valid(&data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_sample_spec_valid(&data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->channel_map_is_set) {
         if (pa_channel_map_compatible(&data->sink->channel_map, &data->sample_spec))
@@ -378,7 +407,10 @@ int pa_sink_input_new(
             pa_channel_map_init_extend(&data->channel_map, data->sample_spec.channels, PA_CHANNEL_MAP_DEFAULT);
     }
 
-    pa_return_val_if_fail(pa_channel_map_compatible(&data->channel_map, &data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_channel_map_compatible(&data->channel_map, &data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     /* Don't restore (or save) stream volume for passthrough streams and
      * prevent attenuation/gain */
@@ -398,19 +430,30 @@ int pa_sink_input_new(
     if (!data->volume_writable)
         data->save_volume = false;
 
-    pa_return_val_if_fail(pa_cvolume_compatible(&data->volume, &data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_cvolume_compatible(&data->volume, &data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->muted_is_set)
         data->muted = false;
 
     if (data->flags & PA_SINK_INPUT_FIX_FORMAT) {
-        pa_return_val_if_fail(pa_format_info_is_pcm(data->format), -PA_ERR_INVALID);
+        if (!pa_format_info_is_pcm(data->format)) {
+            ret = -PA_ERR_INVALID;
+            goto fail;
+        }
+
         data->sample_spec.format = data->sink->sample_spec.format;
         pa_format_info_set_sample_format(data->format, data->sample_spec.format);
     }
 
     if (data->flags & PA_SINK_INPUT_FIX_RATE) {
-        pa_return_val_if_fail(pa_format_info_is_pcm(data->format), -PA_ERR_INVALID);
+        if (!pa_format_info_is_pcm(data->format)) {
+            ret = -PA_ERR_INVALID;
+            goto fail;
+        }
+
         data->sample_spec.rate = data->sink->sample_spec.rate;
         pa_format_info_set_rate(data->format, data->sample_spec.rate);
     }
@@ -418,7 +461,11 @@ int pa_sink_input_new(
     original_cm = data->channel_map;
 
     if (data->flags & PA_SINK_INPUT_FIX_CHANNELS) {
-        pa_return_val_if_fail(pa_format_info_is_pcm(data->format), -PA_ERR_INVALID);
+        if (!pa_format_info_is_pcm(data->format)) {
+            ret = -PA_ERR_INVALID;
+            goto fail;
+        }
+
         data->sample_spec.channels = data->sink->sample_spec.channels;
         data->channel_map = data->sink->channel_map;
         pa_format_info_set_channels(data->format, data->sample_spec.channels);
@@ -443,7 +490,8 @@ int pa_sink_input_new(
         /* rate update failed, or other parts of sample spec didn't match */
 
         pa_log_debug("Could not update sink sample spec to match passthrough stream");
-        return -PA_ERR_NOTSUPPORTED;
+        ret = -PA_ERR_NOTSUPPORTED;
+        goto fail;
     }
 
     /* Due to the fixing of the sample spec the volume might not match anymore */
@@ -452,20 +500,27 @@ int pa_sink_input_new(
     if (data->resample_method == PA_RESAMPLER_INVALID)
         data->resample_method = core->resample_method;
 
-    pa_return_val_if_fail(data->resample_method < PA_RESAMPLER_MAX, -PA_ERR_INVALID);
+    if (data->resample_method >= PA_RESAMPLER_MAX) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
-    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SINK_INPUT_FIXATE], data)) < 0)
-        return r;
+    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SINK_INPUT_FIXATE], data)) < 0) {
+        ret = r;
+        goto fail;
+    }
 
     if ((data->flags & PA_SINK_INPUT_NO_CREATE_ON_SUSPEND) &&
         pa_sink_get_state(data->sink) == PA_SINK_SUSPENDED) {
         pa_log_warn("Failed to create sink input: sink is suspended.");
-        return -PA_ERR_BADSTATE;
+        ret = -PA_ERR_BADSTATE;
+        goto fail;
     }
 
     if (pa_idxset_size(data->sink->inputs) >= PA_MAX_INPUTS_PER_SINK) {
         pa_log_warn("Failed to create sink input: too many inputs per sink.");
-        return -PA_ERR_TOOLARGE;
+        ret = -PA_ERR_TOOLARGE;
+        goto fail;
     }
 
     if ((data->flags & PA_SINK_INPUT_VARIABLE_RATE) ||
@@ -484,7 +539,8 @@ int pa_sink_input_new(
                           (core->disable_remixing || (data->flags & PA_SINK_INPUT_NO_REMIX) ? PA_RESAMPLER_NO_REMIX : 0) |
                           (core->disable_lfe_remixing ? PA_RESAMPLER_NO_LFE : 0)))) {
                 pa_log_warn("Unsupported resampling operation.");
-                return -PA_ERR_NOTSUPPORTED;
+                ret = -PA_ERR_NOTSUPPORTED;
+                goto fail;
             }
     }
 
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index b98118f..3d2cd4a 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -256,20 +256,33 @@ int pa_source_output_new(
         pa_source_output_new_data_set_formats(data, tmp);
     }
 
-    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_NEW], data)) < 0)
-        return r;
+    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_NEW], data)) < 0) {
+        ret = r;
+        goto fail;
+    }
 
-    pa_return_val_if_fail(!data->driver || pa_utf8_valid(data->driver), -PA_ERR_INVALID);
+    if (data->driver && !pa_utf8_valid(data->driver)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->source) {
         pa_source *source;
 
         if (data->direct_on_input) {
             source = data->direct_on_input->sink->monitor_source;
-            pa_return_val_if_fail(source, -PA_ERR_INVALID);
+
+            if (!source) {
+                ret = -PA_ERR_INVALID;
+                goto fail;
+            }
         } else {
             source = pa_namereg_get(core, NULL, PA_NAMEREG_SOURCE);
-            pa_return_val_if_fail(source, -PA_ERR_NOENTITY);
+
+            if (!source) {
+                ret = -PA_ERR_NOENTITY;
+                goto fail;
+            }
         }
 
         pa_source_output_new_data_set_source(data, source, false);
@@ -293,23 +306,39 @@ int pa_source_output_new(
         PA_IDXSET_FOREACH(format, data->req_formats, idx)
             pa_log_info(" -- %s", pa_format_info_snprint(fmt, sizeof(fmt), format));
 
-        return -PA_ERR_NOTSUPPORTED;
+        ret = -PA_ERR_NOTSUPPORTED;
+        goto fail;
     }
 
     /* Now populate the sample spec and format according to the final
      * format that we've negotiated */
-    pa_return_val_if_fail(pa_format_info_to_sample_spec(data->format, &ss, &map) == 0, -PA_ERR_INVALID);
+
+    if (pa_format_info_to_sample_spec(data->format, &ss, &map) < 0) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
+
     pa_source_output_new_data_set_sample_spec(data, &ss);
     if (pa_format_info_is_pcm(data->format) && pa_channel_map_valid(&map))
         pa_source_output_new_data_set_channel_map(data, &map);
 
-    pa_return_val_if_fail(PA_SOURCE_IS_LINKED(pa_source_get_state(data->source)), -PA_ERR_BADSTATE);
-    pa_return_val_if_fail(!data->direct_on_input || data->direct_on_input->sink == data->source->monitor_of, -PA_ERR_INVALID);
+    if (!PA_SOURCE_IS_LINKED(pa_source_get_state(data->source))) {
+        ret = -PA_ERR_BADSTATE;
+        goto fail;
+    }
+
+    if (data->direct_on_input && data->direct_on_input->sink != data->source->monitor_of) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->sample_spec_is_set)
         data->sample_spec = data->source->sample_spec;
 
-    pa_return_val_if_fail(pa_sample_spec_valid(&data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_sample_spec_valid(&data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->channel_map_is_set) {
         if (pa_channel_map_compatible(&data->source->channel_map, &data->sample_spec))
@@ -318,7 +347,10 @@ int pa_source_output_new(
             pa_channel_map_init_extend(&data->channel_map, data->sample_spec.channels, PA_CHANNEL_MAP_DEFAULT);
     }
 
-    pa_return_val_if_fail(pa_channel_map_compatible(&data->channel_map, &data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_channel_map_compatible(&data->channel_map, &data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     /* Don't restore (or save) stream volume for passthrough streams and
      * prevent attenuation/gain */
@@ -338,29 +370,46 @@ int pa_source_output_new(
     if (!data->volume_writable)
         data->save_volume = false;
 
-    pa_return_val_if_fail(pa_cvolume_compatible(&data->volume, &data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_cvolume_compatible(&data->volume, &data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->volume_factor_is_set)
         pa_cvolume_reset(&data->volume_factor, data->sample_spec.channels);
 
-    pa_return_val_if_fail(pa_cvolume_compatible(&data->volume_factor, &data->sample_spec), -PA_ERR_INVALID);
+    if (!pa_cvolume_compatible(&data->volume_factor, &data->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->volume_factor_source_is_set)
         pa_cvolume_reset(&data->volume_factor_source, data->source->sample_spec.channels);
 
-    pa_return_val_if_fail(pa_cvolume_compatible(&data->volume_factor_source, &data->source->sample_spec), -PA_ERR_INVALID);
+    if (!pa_cvolume_compatible(&data->volume_factor_source, &data->source->sample_spec)) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
     if (!data->muted_is_set)
         data->muted = false;
 
     if (data->flags & PA_SOURCE_OUTPUT_FIX_FORMAT) {
-        pa_return_val_if_fail(pa_format_info_is_pcm(data->format), -PA_ERR_INVALID);
+        if (!pa_format_info_is_pcm(data->format)) {
+            ret = -PA_ERR_INVALID;
+            goto fail;
+        }
+
         data->sample_spec.format = data->source->sample_spec.format;
         pa_format_info_set_sample_format(data->format, data->sample_spec.format);
     }
 
     if (data->flags & PA_SOURCE_OUTPUT_FIX_RATE) {
-        pa_return_val_if_fail(pa_format_info_is_pcm(data->format), -PA_ERR_INVALID);
+        if (!pa_format_info_is_pcm(data->format)) {
+            ret = -PA_ERR_INVALID;
+            goto fail;
+        }
+
         pa_format_info_set_rate(data->format, data->sample_spec.rate);
         data->sample_spec.rate = data->source->sample_spec.rate;
     }
@@ -368,7 +417,11 @@ int pa_source_output_new(
     original_cm = data->channel_map;
 
     if (data->flags & PA_SOURCE_OUTPUT_FIX_CHANNELS) {
-        pa_return_val_if_fail(pa_format_info_is_pcm(data->format), -PA_ERR_INVALID);
+        if (!pa_format_info_is_pcm(data->format)) {
+            ret = -PA_ERR_INVALID;
+            goto fail;
+        }
+
         data->sample_spec.channels = data->source->sample_spec.channels;
         data->channel_map = data->source->channel_map;
         pa_format_info_set_channels(data->format, data->sample_spec.channels);
@@ -393,7 +446,8 @@ int pa_source_output_new(
         /* rate update failed, or other parts of sample spec didn't match */
 
         pa_log_debug("Could not update source sample spec to match passthrough stream");
-        return -PA_ERR_NOTSUPPORTED;
+        ret = -PA_ERR_NOTSUPPORTED;
+        goto fail;
     }
 
     /* Due to the fixing of the sample spec the volume might not match anymore */
@@ -402,20 +456,27 @@ int pa_source_output_new(
     if (data->resample_method == PA_RESAMPLER_INVALID)
         data->resample_method = core->resample_method;
 
-    pa_return_val_if_fail(data->resample_method < PA_RESAMPLER_MAX, -PA_ERR_INVALID);
+    if (data->resample_method >= PA_RESAMPLER_MAX) {
+        ret = -PA_ERR_INVALID;
+        goto fail;
+    }
 
-    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_FIXATE], data)) < 0)
-        return r;
+    if ((r = pa_hook_fire(&core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_FIXATE], data)) < 0) {
+        ret = r;
+        goto fail;
+    }
 
     if ((data->flags & PA_SOURCE_OUTPUT_NO_CREATE_ON_SUSPEND) &&
         pa_source_get_state(data->source) == PA_SOURCE_SUSPENDED) {
         pa_log("Failed to create source output: source is suspended.");
-        return -PA_ERR_BADSTATE;
+        ret = -PA_ERR_BADSTATE;
+        goto fail;
     }
 
     if (pa_idxset_size(data->source->outputs) >= PA_MAX_OUTPUTS_PER_SOURCE) {
         pa_log("Failed to create source output: too many outputs per source.");
-        return -PA_ERR_TOOLARGE;
+        ret = -PA_ERR_TOOLARGE;
+        goto fail;
     }
 
     if ((data->flags & PA_SOURCE_OUTPUT_VARIABLE_RATE) ||
@@ -433,7 +494,8 @@ int pa_source_output_new(
                         (core->disable_remixing || (data->flags & PA_SOURCE_OUTPUT_NO_REMIX) ? PA_RESAMPLER_NO_REMIX : 0) |
                         (core->disable_lfe_remixing ? PA_RESAMPLER_NO_LFE : 0)))) {
                 pa_log_warn("Unsupported resampling operation.");
-                return -PA_ERR_NOTSUPPORTED;
+                ret = -PA_ERR_NOTSUPPORTED;
+                goto fail;
             }
     }
 
-- 
1.8.3.1



More information about the pulseaudio-discuss mailing list