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 channel count values. --- src/daemon/daemon-conf.c | 2 +- src/map-file | 1 + src/modules/alsa/alsa-ucm.c | 6 +++--- src/modules/bluetooth/module-bluez4-device.c | 2 +- src/modules/jack/module-jack-sink.c | 3 +-- src/modules/jack/module-jack-source.c | 3 +-- src/modules/jack/module-jackdbus-detect.c | 2 +- src/pulse/channelmap.c | 8 +++----- src/pulse/sample.c | 7 +++++-- src/pulse/sample.h | 4 ++++ src/pulse/volume.c | 5 ++--- src/pulsecore/modargs.c | 3 +-- 12 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c index de3bdc4..ce777a6 100644 --- a/src/daemon/daemon-conf.c +++ b/src/daemon/daemon-conf.c @@ -387,7 +387,7 @@ static int parse_sample_channels(pa_config_parser_state *state) { i = state->data; - if (pa_atoi(state->rvalue, &n) < 0 || n > (int32_t) PA_CHANNELS_MAX || n <= 0) { + if (pa_atoi(state->rvalue, &n) < 0 || !pa_channels_valid(n)) { pa_log(_("[%s:%u] Invalid sample channels '%s'."), state->filename, state->lineno, state->rvalue); return -1; } diff --git a/src/map-file b/src/map-file index 071c141..fbad1a4 100644 --- a/src/map-file +++ b/src/map-file @@ -25,6 +25,7 @@ pa_channel_map_valid; pa_channel_position_from_string; pa_channel_position_to_pretty_string; pa_channel_position_to_string; +pa_channels_valid; pa_context_add_autoload; pa_context_connect; pa_context_disconnect; diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c index 0ed2470..c88fc77 100644 --- a/src/modules/alsa/alsa-ucm.c +++ b/src/modules/alsa/alsa-ucm.c @@ -214,7 +214,7 @@ static int ucm_get_device_property( value = pa_proplist_gets(device->proplist, PA_ALSA_PROP_UCM_PLAYBACK_CHANNELS); if (value) { /* output */ /* get channels */ - if (pa_atou(value, &ui) == 0 && ui < PA_CHANNELS_MAX) + if (pa_atou(value, &ui) == 0 && pa_channels_valid(ui)) device->playback_channels = ui; else pa_log("UCM playback channels %s for device %s out of range", value, device_name); @@ -234,7 +234,7 @@ static int ucm_get_device_property( value = pa_proplist_gets(device->proplist, PA_ALSA_PROP_UCM_CAPTURE_CHANNELS); if (value) { /* input */ /* get channels */ - if (pa_atou(value, &ui) == 0 && ui < PA_CHANNELS_MAX) + if (pa_atou(value, &ui) == 0 && pa_channels_valid(ui)) device->capture_channels = ui; else pa_log("UCM capture channels %s for device %s out of range", value, device_name); @@ -1128,7 +1128,7 @@ static void alsa_mapping_add_ucm_modifier(pa_alsa_mapping *m, pa_alsa_ucm_modifi /* FIXME: channel_str is unsanitized input from the UCM configuration, * we should do proper error handling instead of asserting. * https://bugs.freedesktop.org/show_bug.cgi?id=71823 */ - pa_assert_se(pa_atou(channel_str, &channels) == 0 && channels < PA_CHANNELS_MAX); + pa_assert_se(pa_atou(channel_str, &channels) == 0 && pa_channels_valid(channels)); pa_log_debug("Got channel count %" PRIu32 " for modifier", channels); } diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c index f419cb9..83e603f 100644 --- a/src/modules/bluetooth/module-bluez4-device.c +++ b/src/modules/bluetooth/module-bluez4-device.c @@ -2463,7 +2463,7 @@ int pa__init(pa_module *m) { channels = u->sample_spec.channels; if (pa_modargs_get_value_u32(ma, "channels", &channels) < 0 || - channels <= 0 || channels > PA_CHANNELS_MAX) { + !pa_channels_valid(channels)) { pa_log_error("Failed to get channels from module arguments"); goto fail; } diff --git a/src/modules/jack/module-jack-sink.c b/src/modules/jack/module-jack-sink.c index dccf032..ab97758 100644 --- a/src/modules/jack/module-jack-sink.c +++ b/src/modules/jack/module-jack-sink.c @@ -350,8 +350,7 @@ int pa__init(pa_module*m) { channels = m->core->default_sample_spec.channels; if (pa_modargs_get_value_u32(ma, "channels", &channels) < 0 || - channels <= 0 || - channels > PA_CHANNELS_MAX) { + !pa_channels_valid(channels)) { pa_log("Failed to parse channels= argument."); goto fail; } diff --git a/src/modules/jack/module-jack-source.c b/src/modules/jack/module-jack-source.c index 8f550e1..5d9668b 100644 --- a/src/modules/jack/module-jack-source.c +++ b/src/modules/jack/module-jack-source.c @@ -297,8 +297,7 @@ int pa__init(pa_module*m) { channels = m->core->default_sample_spec.channels; if (pa_modargs_get_value_u32(ma, "channels", &channels) < 0 || - channels <= 0 || - channels >= PA_CHANNELS_MAX) { + !pa_channels_valid(channels)) { pa_log("failed to parse channels= argument."); goto fail; } diff --git a/src/modules/jack/module-jackdbus-detect.c b/src/modules/jack/module-jackdbus-detect.c index 5c29772..160794b 100644 --- a/src/modules/jack/module-jackdbus-detect.c +++ b/src/modules/jack/module-jackdbus-detect.c @@ -238,7 +238,7 @@ int pa__init(pa_module *m) { goto fail; } - if (pa_modargs_get_value_u32(ma, "channels", &u->channels) < 0 || u->channels > PA_CHANNELS_MAX) { + if (pa_modargs_get_value_u32(ma, "channels", &u->channels) < 0 || !pa_channels_valid(u->channels)) { pa_log("Failed to parse channels= argument."); goto fail; } diff --git a/src/pulse/channelmap.c b/src/pulse/channelmap.c index fec0623..72e4130 100644 --- a/src/pulse/channelmap.c +++ b/src/pulse/channelmap.c @@ -199,8 +199,7 @@ pa_channel_map* pa_channel_map_init_stereo(pa_channel_map *m) { pa_channel_map* pa_channel_map_init_auto(pa_channel_map *m, unsigned channels, pa_channel_map_def_t def) { pa_assert(m); - pa_assert(channels > 0); - pa_assert(channels <= PA_CHANNELS_MAX); + pa_assert(pa_channels_valid(channels)); pa_assert(def < PA_CHANNEL_MAP_DEF_MAX); pa_channel_map_init(m); @@ -401,8 +400,7 @@ pa_channel_map* pa_channel_map_init_extend(pa_channel_map *m, unsigned channels, unsigned c; pa_assert(m); - pa_assert(channels > 0); - pa_assert(channels <= PA_CHANNELS_MAX); + pa_assert(pa_channels_valid(channels)); pa_assert(def < PA_CHANNEL_MAP_DEF_MAX); pa_channel_map_init(m); @@ -617,7 +615,7 @@ int pa_channel_map_valid(const pa_channel_map *map) { pa_assert(map); - if (map->channels <= 0 || map->channels > PA_CHANNELS_MAX) + if (!pa_channels_valid(map->channels)) return 0; for (c = 0; c < map->channels; c++) diff --git a/src/pulse/sample.c b/src/pulse/sample.c index 70f7f72..82c1b01 100644 --- a/src/pulse/sample.c +++ b/src/pulse/sample.c @@ -111,12 +111,15 @@ int pa_sample_rate_valid(uint32_t rate) { return rate > 0 && rate <= PA_RATE_MAX; } +int pa_channels_valid(uint8_t channels) { + return channels > 0 && channels <= PA_CHANNELS_MAX; +} + int pa_sample_spec_valid(const pa_sample_spec *spec) { pa_assert(spec); if (PA_UNLIKELY(!pa_sample_rate_valid(spec->rate) || - spec->channels <= 0 || - spec->channels > PA_CHANNELS_MAX || + !pa_channels_valid(spec->channels) || !pa_sample_format_valid(spec->format))) return 0; diff --git a/src/pulse/sample.h b/src/pulse/sample.h index b8abc1c..86973d4 100644 --- a/src/pulse/sample.h +++ b/src/pulse/sample.h @@ -295,6 +295,10 @@ 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 if the channel count is within the supported range. + * \since 5.0 */ +int pa_channels_valid(uint8_t channels) 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/volume.c b/src/pulse/volume.c index fd67254..6927392 100644 --- a/src/pulse/volume.c +++ b/src/pulse/volume.c @@ -73,8 +73,7 @@ pa_cvolume* pa_cvolume_set(pa_cvolume *a, unsigned channels, pa_volume_t v) { int i; pa_assert(a); - pa_assert(channels > 0); - pa_assert(channels <= PA_CHANNELS_MAX); + pa_assert(pa_channels_valid(channels)); a->channels = (uint8_t) channels; @@ -533,7 +532,7 @@ int pa_cvolume_valid(const pa_cvolume *v) { pa_assert(v); - if (v->channels <= 0 || v->channels > PA_CHANNELS_MAX) + if (!pa_channels_valid(v->channels)) return 0; for (c = 0; c < v->channels; c++) diff --git a/src/pulsecore/modargs.c b/src/pulsecore/modargs.c index 15776a0..06973cb 100644 --- a/src/pulsecore/modargs.c +++ b/src/pulsecore/modargs.c @@ -391,8 +391,7 @@ int pa_modargs_get_sample_spec(pa_modargs *ma, pa_sample_spec *rss) { channels = ss.channels; if ((pa_modargs_get_value_u32(ma, "channels", &channels)) < 0 || - channels <= 0 || - channels >= PA_CHANNELS_MAX) + !pa_channels_valid(channels)) return -1; ss.channels = (uint8_t) channels; -- 1.8.3.1