Merge pull request #206 from dvdli/tinyalsa-fix-pcm

Fix compatible issue and refine null parameters checking 

3 files changed, 124 insertions, 33 deletions

diff --git a/src/mixer.c b/src/mixer.c
index a45502e..5581e5d 100644
--- a/src/mixer.c
+++ b/src/mixer.c
@@ -384,6 +384,10 @@ int mixer_add_new_ctls(struct mixer *mixer)
  */
 const char *mixer_get_name(const struct mixer *mixer)
 {
+    if (!mixer) {
+        return NULL;
+    }
+
     return (const char *)mixer->card_info.name;
 }
 
@@ -413,8 +417,9 @@ unsigned int mixer_get_num_ctls_by_name(const struct mixer *mixer, const char *n
     unsigned int count = 0;
     struct mixer_ctl *ctl;
 
-    if (!mixer)
+    if (!mixer || !name) {
         return 0;
+    }
 
     if (mixer->h_grp) {
         grp = mixer->h_grp;
@@ -449,6 +454,10 @@ int mixer_subscribe_events(struct mixer *mixer, int subscribe)
 {
     struct mixer_ctl_group *grp;
 
+    if (!mixer) {
+        return -EINVAL;
+    }
+
     if (mixer->h_grp) {
         grp = mixer->h_grp;
         if (grp->ops->ioctl(grp->data, SNDRV_CTL_IOCTL_SUBSCRIBE_EVENTS, &subscribe) < 0)
@@ -479,6 +488,10 @@ int mixer_wait_event(struct mixer *mixer, int timeout)
     struct mixer_ctl_group *grp;
     int count = 0, num_fds = 0, i, ret = 0;
 
+    if (!mixer) {
+        return -EINVAL;
+    }
+
     if (mixer->fd >= 0)
         num_fds++;
 
@@ -559,6 +572,9 @@ exit:
 int mixer_consume_event(struct mixer *mixer)
 {
     struct mixer_ctl_event ev;
+    if (!mixer) {
+        return -EINVAL;
+    }
 
     return mixer_read_event(mixer, &ev);
 }
@@ -684,6 +700,10 @@ struct mixer_ctl *mixer_get_ctl(struct mixer *mixer, unsigned int id)
  */
 struct mixer_ctl *mixer_get_ctl_by_name(struct mixer *mixer, const char *name)
 {
+    if (!mixer || !name) {
+        return NULL;
+    }
+
     return mixer_get_ctl_by_name_and_index(mixer, name, 0);
 }
 
@@ -703,8 +723,9 @@ struct mixer_ctl *mixer_get_ctl_by_name_and_index(struct mixer *mixer,
     unsigned int n;
     struct mixer_ctl *ctl;
 
-    if (!mixer)
+    if (!mixer || !name) {
         return NULL;
+    }
 
     if (mixer->h_grp) {
         grp = mixer->h_grp;
@@ -754,6 +775,10 @@ void mixer_ctl_update(struct mixer_ctl *ctl)
  */
 int mixer_ctl_is_access_tlv_rw(const struct mixer_ctl *ctl)
 {
+    if (!ctl) {
+        return 0;
+    }
+
     return (ctl->info.access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE);
 }
 
@@ -959,8 +984,9 @@ int mixer_ctl_get_array(const struct mixer_ctl *ctl, void *array, size_t count)
     size_t size;
     void *source;
 
-    if (!ctl || !count || !array)
+    if (!ctl || !array || count == 0) {
         return -EINVAL;
+    }
 
     grp = ctl->grp;
 
@@ -1042,8 +1068,9 @@ int mixer_ctl_set_value(struct mixer_ctl *ctl, unsigned int id, int value)
     struct snd_ctl_elem_value ev;
     int ret;
 
-    if (!ctl || (id >= ctl->info.count))
+    if (!ctl || id >= ctl->info.count) {
         return -EINVAL;
+    }
 
     grp = ctl->grp;
     memset(&ev, 0, sizeof(ev));
@@ -1058,11 +1085,6 @@ int mixer_ctl_set_value(struct mixer_ctl *ctl, unsigned int id, int value)
         break;
 
     case SNDRV_CTL_ELEM_TYPE_INTEGER:
-        if ((value < mixer_ctl_get_range_min(ctl)) ||
-            (value > mixer_ctl_get_range_max(ctl))) {
-            return -EINVAL;
-        }
-
         ev.value.integer.value[id] = value;
         break;
 
@@ -1098,8 +1120,9 @@ int mixer_ctl_set_array(struct mixer_ctl *ctl, const void *array, size_t count)
     size_t size;
     void *dest;
 
-    if ((!ctl) || !count || !array)
+    if (!ctl || !array || count == 0) {
         return -EINVAL;
+    }
 
     grp = ctl->grp;
 
@@ -1167,8 +1190,9 @@ int mixer_ctl_set_array(struct mixer_ctl *ctl, const void *array, size_t count)
  */
 int mixer_ctl_get_range_min(const struct mixer_ctl *ctl)
 {
-    if (!ctl || (ctl->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER))
+    if (!ctl || ctl->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER) {
         return -EINVAL;
+    }
 
     return ctl->info.value.integer.min;
 }
@@ -1183,8 +1207,9 @@ int mixer_ctl_get_range_min(const struct mixer_ctl *ctl)
  */
 int mixer_ctl_get_range_max(const struct mixer_ctl *ctl)
 {
-    if (!ctl || (ctl->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER))
+    if (!ctl || ctl->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER) {
         return -EINVAL;
+    }
 
     return ctl->info.value.integer.max;
 }
@@ -1196,13 +1221,14 @@ int mixer_ctl_get_range_max(const struct mixer_ctl *ctl)
  */
 unsigned int mixer_ctl_get_num_enums(const struct mixer_ctl *ctl)
 {
-    if (!ctl)
+    if (!ctl) {
         return 0;
+    }
 
     return ctl->info.value.enumerated.items;
 }
 
-int mixer_ctl_fill_enum_string(struct mixer_ctl *ctl)
+static int mixer_ctl_fill_enum_string(struct mixer_ctl *ctl)
 {
     struct mixer_ctl_group *grp = ctl->grp;
     struct snd_ctl_elem_info tmp;
@@ -1250,10 +1276,14 @@ fail:
 const char *mixer_ctl_get_enum_string(struct mixer_ctl *ctl,
                                       unsigned int enum_id)
 {
-    if (!ctl || (ctl->info.type != SNDRV_CTL_ELEM_TYPE_ENUMERATED) ||
-        (enum_id >= ctl->info.value.enumerated.items) ||
-        mixer_ctl_fill_enum_string(ctl) != 0)
+    if (!ctl || ctl->info.type != SNDRV_CTL_ELEM_TYPE_ENUMERATED ||
+            enum_id >= ctl->info.value.enumerated.items) {
+        return NULL;
+    }
+
+    if (mixer_ctl_fill_enum_string(ctl) < 0) {
         return NULL;
+    }
 
     return (const char *)ctl->ename[enum_id];
 }
@@ -1272,9 +1302,13 @@ int mixer_ctl_set_enum_by_string(struct mixer_ctl *ctl, const char *string)
     struct snd_ctl_elem_value ev;
     int ret;
 
-    if (!ctl || (ctl->info.type != SNDRV_CTL_ELEM_TYPE_ENUMERATED) ||
-        mixer_ctl_fill_enum_string(ctl) != 0)
+    if (!ctl || !string || ctl->info.type != SNDRV_CTL_ELEM_TYPE_ENUMERATED) {
         return -EINVAL;
+    }
+
+    if (mixer_ctl_fill_enum_string(ctl) < 0) {
+        return -EINVAL;
+    }
 
     grp = ctl->grp;
     num_enums = ctl->info.value.enumerated.items;
diff --git a/src/pcm.c b/src/pcm.c
index 63ca65f..a97d325 100644
--- a/src/pcm.c
+++ b/src/pcm.c
@@ -623,6 +623,16 @@ static int pcm_sync_ptr(struct pcm *pcm, int flags)
     return 0;
 }
 
+int pcm_state(struct pcm *pcm)
+{
+    // Update the state only. Do not sync HW sync.
+    int err = pcm_sync_ptr(pcm, SNDRV_PCM_SYNC_PTR_APPL | SNDRV_PCM_SYNC_PTR_AVAIL_MIN);
+    if (err < 0)
+        return err;
+
+    return pcm->mmap_status->state;
+}
+
 static int pcm_hw_mmap_status(struct pcm *pcm)
 {
     if (pcm->sync_ptr)
@@ -1196,6 +1206,10 @@ int pcm_prepare(struct pcm *pcm)
  */
 int pcm_start(struct pcm *pcm)
 {
+    if (pcm_state(pcm) == PCM_STATE_SETUP && pcm_prepare(pcm) != 0) {
+        return -1;
+    }
+
     /* set appl_ptr and avail_min in kernel */
     if (pcm_sync_ptr(pcm, 0) < 0)
         return -1;
@@ -1409,16 +1423,6 @@ again:
     return 0;
 }
 
-int pcm_state(struct pcm *pcm)
-{
-    // Update the state only. Do not sync HW sync.
-    int err = pcm_sync_ptr(pcm, SNDRV_PCM_SYNC_PTR_APPL | SNDRV_PCM_SYNC_PTR_AVAIL_MIN);
-    if (err < 0)
-        return err;
-
-    return pcm->mmap_status->state;
-}
-
 /** Waits for frames to be available for read or write operations.
  * @param pcm A PCM handle.
  * @param timeout The maximum amount of time to wait for, in terms of milliseconds.
diff --git a/tests/src/mixer_test.cc b/tests/src/mixer_test.cc
index 903fb8f..cbce3ec 100644
--- a/tests/src/mixer_test.cc
+++ b/tests/src/mixer_test.cc
@@ -27,6 +27,7 @@
 */
 #include "pcm_test_device.h"
 
+#include <limits>
 #include <string_view>
 #include <string>
 #include <thread>
@@ -51,8 +52,60 @@ static constexpr int k100Percent = 100;
 static constexpr int k0Percent = 0;
 
 TEST(MixerTest, OpenAndClose) {
-    ASSERT_EQ(mixer_open(1000), nullptr);
+    // assume card 0 is always probed.
+    mixer *mixer_object = mixer_open(0);
+    EXPECT_NE(mixer_object, nullptr);
+    mixer_close(mixer_object);
+}
+
+TEST(MixerTest, NullParametersCheck) {
+    EXPECT_EQ(mixer_open(1000), nullptr);
     mixer_close(nullptr);
+    EXPECT_EQ(mixer_add_new_ctls(nullptr), 0);
+    EXPECT_EQ(mixer_get_name(nullptr), nullptr);
+    EXPECT_EQ(mixer_get_num_ctls(nullptr), 0);
+    EXPECT_EQ(mixer_get_num_ctls_by_name(nullptr, ""), 0);
+    EXPECT_EQ(mixer_get_num_ctls_by_name(reinterpret_cast<const mixer *>(1), nullptr), 0);
+    EXPECT_EQ(mixer_get_ctl_const(nullptr, 0), nullptr);
+    EXPECT_EQ(mixer_get_ctl(nullptr, 0), nullptr);
+    EXPECT_EQ(mixer_get_ctl_by_name(nullptr, ""), nullptr);
+    EXPECT_EQ(mixer_get_ctl_by_name(reinterpret_cast<mixer *>(1), nullptr), nullptr);
+    EXPECT_EQ(mixer_get_ctl_by_name_and_index(nullptr, "", 0), nullptr);
+    EXPECT_EQ(
+            mixer_get_ctl_by_name_and_index(reinterpret_cast<mixer *>(1), nullptr, 0),
+            nullptr);
+    EXPECT_NE(mixer_subscribe_events(nullptr, 0), 0);
+    EXPECT_LT(mixer_wait_event(nullptr, 0), 0);
+    EXPECT_EQ(mixer_ctl_get_id(nullptr), std::numeric_limits<unsigned int>::max());
+    EXPECT_EQ(mixer_ctl_get_name(nullptr), nullptr);
+    EXPECT_EQ(mixer_ctl_get_type(nullptr), MIXER_CTL_TYPE_UNKNOWN);
+    EXPECT_STREQ(mixer_ctl_get_type_string(nullptr), "");
+    EXPECT_EQ(mixer_ctl_get_num_values(nullptr), 0);
+    EXPECT_EQ(mixer_ctl_get_num_enums(nullptr), 0);
+    EXPECT_EQ(mixer_ctl_get_enum_string(nullptr, 0), nullptr);
+    mixer_ctl_update(nullptr);
+    EXPECT_EQ(mixer_ctl_is_access_tlv_rw(nullptr), 0);
+    EXPECT_EQ(mixer_ctl_get_percent(nullptr, 0), -EINVAL);
+    EXPECT_EQ(mixer_ctl_set_percent(nullptr, 0, 0), -EINVAL);
+    EXPECT_EQ(mixer_ctl_get_value(nullptr, 0), -EINVAL);
+    EXPECT_EQ(mixer_ctl_get_array(nullptr, reinterpret_cast<void *>(1), 1), -EINVAL);
+    EXPECT_EQ(mixer_ctl_get_array(reinterpret_cast<mixer_ctl *>(1), nullptr, 1), -EINVAL);
+    EXPECT_EQ(
+            mixer_ctl_get_array(
+                reinterpret_cast<mixer_ctl *>(1), reinterpret_cast<void *>(1), 0), -EINVAL);
+    EXPECT_EQ(mixer_ctl_set_value(nullptr, 0, 0), -EINVAL);
+    EXPECT_EQ(mixer_ctl_set_array(nullptr, reinterpret_cast<const void *>(1), 1), -EINVAL);
+    EXPECT_EQ(mixer_ctl_set_array(reinterpret_cast<mixer_ctl *>(1), nullptr, 1), -EINVAL);
+    EXPECT_EQ(
+            mixer_ctl_set_array(
+                reinterpret_cast<mixer_ctl *>(1), reinterpret_cast<const void *>(1), 0), -EINVAL);
+    EXPECT_EQ(mixer_ctl_set_enum_by_string(nullptr, reinterpret_cast<const char *>(1)), -EINVAL);
+    EXPECT_EQ(mixer_ctl_set_enum_by_string(reinterpret_cast<mixer_ctl *>(1), nullptr), -EINVAL);
+    EXPECT_EQ(mixer_ctl_get_range_min(nullptr), -EINVAL);
+    EXPECT_EQ(mixer_ctl_get_range_max(nullptr), -EINVAL);
+    EXPECT_EQ(mixer_read_event(nullptr, reinterpret_cast<mixer_ctl_event *>(1)), -EINVAL);
+    EXPECT_EQ(mixer_read_event(reinterpret_cast<mixer *>(1), nullptr), -EINVAL);
+    EXPECT_EQ(mixer_consume_event(nullptr), -EINVAL);
 }
 
 class MixerTest : public ::testing::TestWithParam<unsigned int> {
@@ -105,9 +158,9 @@ class MixerControlsTest : public MixerTest {
 
         for (unsigned int i = 0; i < number_of_controls; i++) {
             controls[i] = mixer_get_ctl_const(mixer_object, i);
-            ASSERT_EQ(mixer_ctl_get_id(controls[i]), i);
-            ASSERT_STRNE(mixer_ctl_get_name(controls[i]), "");
-            ASSERT_NE(controls[i], nullptr);
+            EXPECT_EQ(mixer_ctl_get_id(controls[i]), i);
+            EXPECT_STRNE(mixer_ctl_get_name(controls[i]), "");
+            EXPECT_NE(controls[i], nullptr);
         }
     }