[pulseaudio-discuss] [PATCH 02/16] Add pa_sample_rate_valid()

Tanu Kaskinen tanu.kaskinen at linux.intel.com
Tue Dec 3 23:50:10 PST 2013


I think this makes the code a bit nicer to read and write. This also
reduces the chances of off-by-one errors when checking the bounds of
sample rate values.
---
 src/daemon/daemon-conf.c                     |  4 ++--
 src/map-file                                 |  1 +
 src/modules/alsa/alsa-ucm.c                  |  4 ++--
 src/modules/bluetooth/module-bluez4-device.c |  3 +--
 src/modules/dbus/iface-core.c                |  6 +++---
 src/pulse/sample.c                           |  7 +++++--
 src/pulse/sample.h                           |  3 +++
 src/pulse/stream.c                           |  2 +-
 src/pulsecore/modargs.c                      | 10 +++-------
 src/pulsecore/protocol-native.c              |  2 +-
 src/pulsecore/resampler.c                    |  4 ++--
 src/pulsecore/sink.c                         |  3 +--
 src/pulsecore/source.c                       |  3 +--
 13 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
index 5a480cc..de3bdc4 100644
--- a/src/daemon/daemon-conf.c
+++ b/src/daemon/daemon-conf.c
@@ -345,7 +345,7 @@ static int parse_sample_rate(pa_config_parser_state *state) {
 
     c = state->data;
 
-    if (pa_atou(state->rvalue, &r) < 0 || r > (uint32_t) PA_RATE_MAX || r <= 0 ||
+    if (pa_atou(state->rvalue, &r) < 0 || !pa_sample_rate_valid(r) ||
         !((r % 4000 == 0) || (r % 11025 == 0))) {
         pa_log(_("[%s:%u] Invalid sample rate '%s'."), state->filename, state->lineno, state->rvalue);
         return -1;
@@ -363,7 +363,7 @@ static int parse_alternate_sample_rate(pa_config_parser_state *state) {
 
     c = state->data;
 
-    if (pa_atou(state->rvalue, &r) < 0 || r > (uint32_t) PA_RATE_MAX || r <= 0 ||
+    if (pa_atou(state->rvalue, &r) < 0 || !pa_sample_rate_valid(r) ||
         !((r % 4000==0) || (r % 11025 == 0))) {
         pa_log(_("[%s:%u] Invalid sample rate '%s'."), state->filename, state->lineno, state->rvalue);
         return -1;
diff --git a/src/map-file b/src/map-file
index 45f5e41..071c141 100644
--- a/src/map-file
+++ b/src/map-file
@@ -252,6 +252,7 @@ pa_sample_format_is_be;
 pa_sample_format_is_le;
 pa_sample_format_to_string;
 pa_sample_format_valid;
+pa_sample_rate_valid;
 pa_sample_size;
 pa_sample_size_of_format;
 pa_sample_spec_equal;
diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c
index ac7f70d..0ed2470 100644
--- a/src/modules/alsa/alsa-ucm.c
+++ b/src/modules/alsa/alsa-ucm.c
@@ -263,7 +263,7 @@ static int ucm_get_device_property(
         /* get rate */
         if ((value = pa_proplist_gets(device->proplist, PA_ALSA_PROP_UCM_PLAYBACK_RATE)) ||
             (value = pa_proplist_gets(verb->proplist, PA_ALSA_PROP_UCM_PLAYBACK_RATE))) {
-            if (pa_atou(value, &ui) == 0 && ui > 0 && ui < PA_RATE_MAX) {
+            if (pa_atou(value, &ui) == 0 && pa_sample_rate_valid(ui)) {
                 pa_log_debug("UCM playback device %s rate %d", device_name, ui);
                 device->playback_rate = ui;
             } else
@@ -284,7 +284,7 @@ static int ucm_get_device_property(
         /* get rate */
         if ((value = pa_proplist_gets(device->proplist, PA_ALSA_PROP_UCM_CAPTURE_RATE)) ||
             (value = pa_proplist_gets(verb->proplist, PA_ALSA_PROP_UCM_CAPTURE_RATE))) {
-            if (pa_atou(value, &ui) == 0 && ui > 0 && ui < PA_RATE_MAX) {
+            if (pa_atou(value, &ui) == 0 && pa_sample_rate_valid(ui)) {
                 pa_log_debug("UCM capture device %s rate %d", device_name, ui);
                 device->capture_rate = ui;
             } else
diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c
index 047332b..f419cb9 100644
--- a/src/modules/bluetooth/module-bluez4-device.c
+++ b/src/modules/bluetooth/module-bluez4-device.c
@@ -2450,8 +2450,7 @@ int pa__init(pa_module *m) {
         goto fail;
     }
 
-    if (pa_modargs_get_value_u32(ma, "rate", &u->sample_spec.rate) < 0 ||
-        u->sample_spec.rate <= 0 || u->sample_spec.rate > PA_RATE_MAX) {
+    if (pa_modargs_get_sample_rate(ma, &u->sample_spec.rate) < 0) {
         pa_log_error("Failed to get rate from module arguments");
         goto fail;
     }
diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c
index 9c293fd..c7853cc 100644
--- a/src/modules/dbus/iface-core.c
+++ b/src/modules/dbus/iface-core.c
@@ -544,7 +544,7 @@ static void handle_set_default_sample_rate(DBusConnection *conn, DBusMessage *ms
 
     dbus_message_iter_get_basic(iter, &default_sample_rate);
 
-    if (default_sample_rate <= 0 || default_sample_rate > PA_RATE_MAX ||
+    if (!pa_sample_rate_valid(default_sample_rate) ||
         !((default_sample_rate % 4000 == 0) || (default_sample_rate % 11025 == 0)))  {
         pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, "Invalid sample rate.");
         return;
@@ -579,7 +579,7 @@ static void handle_set_alternate_sample_rate(DBusConnection *conn, DBusMessage *
 
     dbus_message_iter_get_basic(iter, &alternate_sample_rate);
 
-    if (alternate_sample_rate <= 0 || alternate_sample_rate > PA_RATE_MAX ||
+    if (!pa_sample_rate_valid(alternate_sample_rate) ||
         !((alternate_sample_rate % 4000 == 0) || (alternate_sample_rate % 11025 == 0))) {
         pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, "Invalid sample rate.");
         return;
@@ -1322,7 +1322,7 @@ static void handle_upload_sample(DBusConnection *conn, DBusMessage *msg, void *u
         goto finish;
     }
 
-    if (sample_rate <= 0 || sample_rate > PA_RATE_MAX) {
+    if (!pa_sample_rate_valid(sample_rate)) {
         pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, "Invalid sample rate.");
         goto finish;
     }
diff --git a/src/pulse/sample.c b/src/pulse/sample.c
index f350380..70f7f72 100644
--- a/src/pulse/sample.c
+++ b/src/pulse/sample.c
@@ -107,11 +107,14 @@ int pa_sample_format_valid(unsigned format) {
     return format < PA_SAMPLE_MAX;
 }
 
+int pa_sample_rate_valid(uint32_t rate) {
+    return rate > 0 && rate <= PA_RATE_MAX;
+}
+
 int pa_sample_spec_valid(const pa_sample_spec *spec) {
     pa_assert(spec);
 
-    if (PA_UNLIKELY (spec->rate <= 0 ||
-        spec->rate > PA_RATE_MAX ||
+    if (PA_UNLIKELY(!pa_sample_rate_valid(spec->rate) ||
         spec->channels <= 0 ||
         spec->channels > PA_CHANNELS_MAX ||
         !pa_sample_format_valid(spec->format)))
diff --git a/src/pulse/sample.h b/src/pulse/sample.h
index 23c7d73..b8abc1c 100644
--- a/src/pulse/sample.h
+++ b/src/pulse/sample.h
@@ -292,6 +292,9 @@ pa_sample_spec* pa_sample_spec_init(pa_sample_spec *spec);
 /** Return non-zero if the given integer is a valid sample format. \since 5.0 */
 int pa_sample_format_valid(unsigned format) PA_GCC_PURE;
 
+/** Return non-zero if the rate is within the supported range. \since 5.0 */
+int pa_sample_rate_valid(uint32_t rate) PA_GCC_PURE;
+
 /** Return non-zero when the sample type specification is valid */
 int pa_sample_spec_valid(const pa_sample_spec *spec) PA_GCC_PURE;
 
diff --git a/src/pulse/stream.c b/src/pulse/stream.c
index 746179b..d376326 100644
--- a/src/pulse/stream.c
+++ b/src/pulse/stream.c
@@ -2785,7 +2785,7 @@ pa_operation *pa_stream_update_sample_rate(pa_stream *s, uint32_t rate, pa_strea
     pa_assert(PA_REFCNT_VALUE(s) >= 1);
 
     PA_CHECK_VALIDITY_RETURN_NULL(s->context, !pa_detect_fork(), PA_ERR_FORKED);
-    PA_CHECK_VALIDITY_RETURN_NULL(s->context, rate > 0 && rate <= PA_RATE_MAX, PA_ERR_INVALID);
+    PA_CHECK_VALIDITY_RETURN_NULL(s->context, pa_sample_rate_valid(rate), PA_ERR_INVALID);
     PA_CHECK_VALIDITY_RETURN_NULL(s->context, s->state == PA_STREAM_READY, PA_ERR_BADSTATE);
     PA_CHECK_VALIDITY_RETURN_NULL(s->context, s->direction != PA_STREAM_UPLOAD, PA_ERR_BADSTATE);
     PA_CHECK_VALIDITY_RETURN_NULL(s->context, s->flags & PA_STREAM_VARIABLE_RATE, PA_ERR_BADSTATE);
diff --git a/src/pulsecore/modargs.c b/src/pulsecore/modargs.c
index 04e60fd..15776a0 100644
--- a/src/pulsecore/modargs.c
+++ b/src/pulsecore/modargs.c
@@ -370,8 +370,7 @@ int pa_modargs_get_sample_rate(pa_modargs *ma, uint32_t *rate) {
     pa_assert(rate);
 
     if ((pa_modargs_get_value_u32(ma, "rate", &rate_local)) < 0 ||
-        rate_local <= 0 ||
-        rate_local > PA_RATE_MAX)
+        !pa_sample_rate_valid(rate_local))
         return -1;
 
     *rate = rate_local;
@@ -387,9 +386,7 @@ int pa_modargs_get_sample_spec(pa_modargs *ma, pa_sample_spec *rss) {
     pa_assert(rss);
 
     ss = *rss;
-    if ((pa_modargs_get_value_u32(ma, "rate", &ss.rate)) < 0 ||
-        ss.rate <= 0 ||
-        ss.rate > PA_RATE_MAX)
+    if ((pa_modargs_get_sample_rate(ma, &ss.rate)) < 0)
         return -1;
 
     channels = ss.channels;
@@ -416,8 +413,7 @@ int pa_modargs_get_alternate_sample_rate(pa_modargs *ma, uint32_t *alternate_rat
     pa_assert(alternate_rate);
 
     if ((pa_modargs_get_value_u32(ma, "alternate_rate", alternate_rate)) < 0 ||
-        *alternate_rate <= 0 ||
-        *alternate_rate > PA_RATE_MAX ||
+        !pa_sample_rate_valid(*alternate_rate) ||
         !((*alternate_rate % 4000 == 0) || (*alternate_rate % 11025 == 0)))
         return -1;
 
diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index c1ff04a..662de98 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -4112,7 +4112,7 @@ static void command_update_stream_sample_rate(pa_pdispatch *pd, uint32_t command
     }
 
     CHECK_VALIDITY(c->pstream, c->authorized, tag, PA_ERR_ACCESS);
-    CHECK_VALIDITY(c->pstream, rate > 0 && rate <= PA_RATE_MAX, tag, PA_ERR_INVALID);
+    CHECK_VALIDITY(c->pstream, pa_sample_rate_valid(rate), tag, PA_ERR_INVALID);
 
     if (command == PA_COMMAND_UPDATE_PLAYBACK_STREAM_SAMPLE_RATE) {
         playback_stream *s;
diff --git a/src/pulsecore/resampler.c b/src/pulsecore/resampler.c
index d98f71d..b385d59 100644
--- a/src/pulsecore/resampler.c
+++ b/src/pulsecore/resampler.c
@@ -203,8 +203,8 @@ static pa_resample_method_t pa_resampler_fix_method(
                 const uint32_t rate_a,
                 const uint32_t rate_b) {
 
-    pa_assert(rate_a > 0 && rate_a <= PA_RATE_MAX);
-    pa_assert(rate_b > 0 && rate_b <= PA_RATE_MAX);
+    pa_assert(pa_sample_rate_valid(rate_a));
+    pa_assert(pa_sample_rate_valid(rate_b));
     pa_assert(method >= 0);
     pa_assert(method < PA_RESAMPLER_MAX);
 
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 95cf9b6..672a884 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -1420,8 +1420,7 @@ int pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
         }
     }
 
-    if (PA_UNLIKELY (desired_rate < 8000 ||
-                     desired_rate > PA_RATE_MAX))
+    if (PA_UNLIKELY(!pa_sample_rate_valid(desired_rate)))
         return -1;
 
     if (!passthrough) {
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index a6dc9bf..6073b1d 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -1009,8 +1009,7 @@ int pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
         }
     }
 
-    if (PA_UNLIKELY (desired_rate < 8000 ||
-                     desired_rate > PA_RATE_MAX))
+    if (PA_UNLIKELY(!pa_sample_rate_valid(desired_rate)))
         return -1;
 
     if (!passthrough) {
-- 
1.8.3.1



More information about the pulseaudio-discuss mailing list