From ceb2ad035678f14e91798dff7492cb686837642a Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 13 Nov 2020 16:26:26 +0900 Subject: [PATCH] ctl: add checks for method arguments In Rules for use of GError, it's just used for recoverable runtime error, not for programming error. The invalid arguments are a kind of programming error. This commit adds the check for method arguments. Signed-off-by: Takashi Sakamoto --- src/ctl/card.c | 79 ++++++++++++++++++++------------------------ src/ctl/elem-id.c | 2 ++ src/ctl/elem-info.c | 39 ++++++++-------------- src/ctl/elem-value.c | 63 +++++++++++++++++++++++++++-------- src/ctl/query.c | 8 ++--- 5 files changed, 103 insertions(+), 88 deletions(-) diff --git a/src/ctl/card.c b/src/ctl/card.c index 32c277d..03fcae6 100644 --- a/src/ctl/card.c +++ b/src/ctl/card.c @@ -235,6 +235,7 @@ void alsactl_card_get_protocol_version(ALSACtlCard *self, g_return_if_fail(ALSACTL_IS_CARD(self)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(proto_ver_triplet != NULL); g_return_if_fail(error == NULL || *error == NULL); if (priv->fd < 0) { @@ -265,6 +266,7 @@ void alsactl_card_get_info(ALSACtlCard *self, ALSACtlCardInfo **card_info, g_return_if_fail(ALSACTL_IS_CARD(self)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(card_info != NULL); g_return_if_fail(error == NULL || *error == NULL); *card_info = g_object_new(ALSACTL_TYPE_CARD_INFO, NULL); @@ -347,6 +349,7 @@ void alsactl_card_get_elem_id_list(ALSACtlCard *self, GList **entries, g_return_if_fail(ALSACTL_IS_CARD(self)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(entries != NULL); g_return_if_fail(error == NULL || *error == NULL); allocate_elem_ids(priv->fd, &list, error); @@ -384,6 +387,7 @@ void alsactl_card_lock_elem(ALSACtlCard *self, const ALSACtlElemId *elem_id, g_return_if_fail(ALSACTL_IS_CARD(self)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); g_return_if_fail(error == NULL || *error == NULL); if (lock) @@ -440,9 +444,10 @@ void alsactl_card_get_elem_info(ALSACtlCard *self, const ALSACtlElemId *elem_id, struct snd_ctl_elem_info *info; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + g_return_if_fail(elem_info != NULL); g_return_if_fail(error == NULL || *error == NULL); *elem_info = g_object_new(ALSACTL_TYPE_ELEM_INFO, NULL); @@ -480,7 +485,7 @@ void alsactl_card_get_elem_info(ALSACtlCard *self, const ALSACtlElemId *elem_id, break; } default: - generate_error(error, ENXIO); + g_return_if_reached(); return; } } @@ -510,16 +515,14 @@ void alsactl_card_write_elem_tlv(ALSACtlCard *self, size_t container_size; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + // At least two quadlets should be included for type and length. + g_return_if_fail(container != NULL); + g_return_if_fail(container_count >= 2); g_return_if_fail(error == NULL || *error == NULL); - // At least two quadlets should be included for type and length. - if (container == NULL || container_count < 2) { - generate_error(error, EINVAL); - return; - } container_size = container_count * sizeof(*container); packet = g_malloc0(sizeof(*packet) + container_size); @@ -558,16 +561,14 @@ void alsactl_card_read_elem_tlv(ALSACtlCard *self, const ALSACtlElemId *elem_id, size_t container_size; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + // At least two quadlets should be included for type and length. + g_return_if_fail(container != NULL); + g_return_if_fail(container_count != NULL && *container_count >= 2); g_return_if_fail(error == NULL || *error == NULL); - // At least two quadlets should be included for type and length. - if (*container == NULL || *container_count < 2) { - generate_error(error, EINVAL); - return; - } container_size = *container_count * sizeof(**container); packet = g_malloc0(sizeof(*packet) + container_size); @@ -609,16 +610,14 @@ void alsactl_card_command_elem_tlv(ALSACtlCard *self, size_t container_size; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + // At least two quadlets should be included for type and length. + g_return_if_fail(container != NULL); + g_return_if_fail(container_count != NULL && *container_count >= 2); g_return_if_fail(error == NULL || *error == NULL); - // At least two quadlets should be included for type and length. - if (*container == NULL || *container_count < 2) { - generate_error(error, EINVAL); - return; - } container_size = *container_count * sizeof(**container); packet = g_malloc0(sizeof(*packet) + container_size); @@ -636,7 +635,7 @@ void alsactl_card_command_elem_tlv(ALSACtlCard *self, g_free(packet); } -static int prepare_enum_names(struct snd_ctl_elem_info *info, const gchar **labels) +static void prepare_enum_names(struct snd_ctl_elem_info *info, const gchar **labels) { unsigned int count; unsigned int length; @@ -646,14 +645,12 @@ static int prepare_enum_names(struct snd_ctl_elem_info *info, const gchar **labe for (count = 0; labels[count] != NULL; ++count) { const gchar *label = labels[count]; - if (strlen(label) >= 64) - return -EINVAL; + g_return_if_fail(strlen(label) < 64); length += strlen(label) + 1; } - if (length > 64 * 1024) - return -EINVAL; + g_return_if_fail(length <= 64 * 1024); pos = g_malloc0(length); @@ -667,8 +664,6 @@ static int prepare_enum_names(struct snd_ctl_elem_info *info, const gchar **labe pos += strlen(label) + 1; } info->value.enumerated.items = count; - - return 0; } static void add_or_replace_elems(int fd, const ALSACtlElemId *elem_id, @@ -692,17 +687,12 @@ static void add_or_replace_elems(int fd, const ALSACtlElemId *elem_id, case SNDRV_CTL_ELEM_TYPE_ENUMERATED: { const gchar **labels; - int err; alsactl_elem_info_get_enum_data(elem_info, &labels, error); if (*error != NULL) return; - err = prepare_enum_names(info, labels); - if (err < 0) { - generate_error(error, -err); - return; - } + prepare_enum_names(info, labels); break; } @@ -754,10 +744,11 @@ void alsactl_card_add_elems(ALSACtlCard *self, const ALSACtlElemId *elem_id, ALSACtlCardPrivate *priv; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); - g_return_if_fail(ALSACTL_IS_ELEM_INFO(elem_info)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + g_return_if_fail(elem_count > 0); + g_return_if_fail(ALSACTL_IS_ELEM_INFO(elem_info)); g_return_if_fail(error == NULL || *error == NULL); add_or_replace_elems(priv->fd, elem_id, elem_count, elem_info, FALSE, @@ -785,10 +776,11 @@ void alsactl_card_replace_elems(ALSACtlCard *self, const ALSACtlElemId *elem_id, ALSACtlCardPrivate *priv; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); - g_return_if_fail(ALSACTL_IS_ELEM_INFO(elem_info)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + g_return_if_fail(elem_count > 0); + g_return_if_fail(ALSACTL_IS_ELEM_INFO(elem_info)); g_return_if_fail(error == NULL || *error == NULL); add_or_replace_elems(priv->fd, elem_id, elem_count, elem_info, TRUE, @@ -812,9 +804,9 @@ void alsactl_card_remove_elems(ALSACtlCard *self, const ALSACtlElemId *elem_id, ALSACtlCardPrivate *priv; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); g_return_if_fail(error == NULL || *error == NULL); if (ioctl(priv->fd, SNDRV_CTL_IOCTL_ELEM_REMOVE, elem_id) < 0) @@ -842,15 +834,15 @@ void alsactl_card_write_elem_value(ALSACtlCard *self, struct snd_ctl_elem_value *value; g_return_if_fail(ALSACTL_IS_CARD(self)); + priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); g_return_if_fail(ALSACTL_IS_ELEM_VALUE(elem_value)); - g_return_if_fail(error == NULL || *error == NULL); ctl_elem_value_refer_private((ALSACtlElemValue *)elem_value, &value); value->id = *elem_id; - priv = alsactl_card_get_instance_private(self); if (ioctl(priv->fd, SNDRV_CTL_IOCTL_ELEM_WRITE, value) < 0) generate_error(error, errno); } @@ -876,15 +868,15 @@ void alsactl_card_read_elem_value(ALSACtlCard *self, struct snd_ctl_elem_value *value; g_return_if_fail(ALSACTL_IS_CARD(self)); - g_return_if_fail(elem_id != NULL); - g_return_if_fail(ALSACTL_IS_ELEM_VALUE(*elem_value)); + priv = alsactl_card_get_instance_private(self); + g_return_if_fail(elem_id != NULL); + g_return_if_fail(elem_value != NULL && ALSACTL_IS_ELEM_VALUE(*elem_value)); g_return_if_fail(error == NULL || *error == NULL); ctl_elem_value_refer_private(*elem_value, &value); value->id = *elem_id; - priv = alsactl_card_get_instance_private(self); if (ioctl(priv->fd, SNDRV_CTL_IOCTL_ELEM_READ, value) < 0) generate_error(error, errno); } @@ -1002,6 +994,7 @@ void alsactl_card_create_source(ALSACtlCard *self, GSource **gsrc, g_return_if_fail(ALSACTL_IS_CARD(self)); priv = alsactl_card_get_instance_private(self); + g_return_if_fail(gsrc != NULL); g_return_if_fail(error == NULL || *error == NULL); if (priv->fd < 0) { diff --git a/src/ctl/elem-id.c b/src/ctl/elem-id.c index 35033e1..fa3a810 100644 --- a/src/ctl/elem-id.c +++ b/src/ctl/elem-id.c @@ -57,6 +57,8 @@ ALSACtlElemId *alsactl_elem_id_new_by_name(ALSACtlElemIfaceType iface, { struct snd_ctl_elem_id *id; + g_return_val_if_fail(name != NULL && strlen(name) > 0, NULL); + id = g_malloc0(sizeof(*id)); id->iface = iface; diff --git a/src/ctl/elem-info.c b/src/ctl/elem-info.c index 60aee6d..a2c30de 100644 --- a/src/ctl/elem-info.c +++ b/src/ctl/elem-info.c @@ -175,8 +175,7 @@ ALSACtlElemInfo *alsactl_elem_info_new(ALSACtlElemType elem_type, GError **error case ALSACTL_ELEM_TYPE_INTEGER64: break; default: - generate_error(error, EINVAL); - return NULL; + g_return_val_if_reached(NULL); } return g_object_new(ALSACTL_TYPE_ELEM_INFO, "type", elem_type, NULL); @@ -202,12 +201,10 @@ void alsactl_elem_info_get_int_data(ALSACtlElemInfo *self, g_return_if_fail(ALSACTL_IS_ELEM_INFO(self)); priv = alsactl_elem_info_get_instance_private(self); + g_return_if_fail(data != NULL); g_return_if_fail(error == NULL || *error == NULL); - if (priv->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER) { - generate_error(error, ENXIO); - return; - } + g_return_if_fail(priv->info.type == SNDRV_CTL_ELEM_TYPE_INTEGER); priv->int_data.min = (gint32)priv->info.value.integer.min; priv->int_data.max = (gint32)priv->info.value.integer.max; @@ -236,12 +233,10 @@ void alsactl_elem_info_set_int_data(ALSACtlElemInfo *self, g_return_if_fail(ALSACTL_IS_ELEM_INFO(self)); priv = alsactl_elem_info_get_instance_private(self); + g_return_if_fail(data != NULL); g_return_if_fail(error == NULL || *error == NULL); - if (priv->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER) { - generate_error(error, ENXIO); - return; - } + g_return_if_fail(priv->info.type == SNDRV_CTL_ELEM_TYPE_INTEGER); priv->info.value.integer.min = (long)data[0]; priv->info.value.integer.max = (long)data[1]; @@ -268,12 +263,10 @@ void alsactl_elem_info_get_int64_data(ALSACtlElemInfo *self, g_return_if_fail(ALSACTL_IS_ELEM_INFO(self)); priv = alsactl_elem_info_get_instance_private(self); + g_return_if_fail(data != NULL); g_return_if_fail(error == NULL || *error == NULL); - if (priv->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER64) { - generate_error(error, ENXIO); - return; - } + g_return_if_fail(priv->info.type == SNDRV_CTL_ELEM_TYPE_INTEGER64); priv->int_data.min = (gint64)priv->info.value.integer.min; priv->int_data.max = (gint64)priv->info.value.integer.max; @@ -302,12 +295,10 @@ void alsactl_elem_info_set_int64_data(ALSACtlElemInfo *self, g_return_if_fail(ALSACTL_IS_ELEM_INFO(self)); priv = alsactl_elem_info_get_instance_private(self); + g_return_if_fail(data != NULL); g_return_if_fail(error == NULL || *error == NULL); - if (priv->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER64) { - generate_error(error, ENXIO); - return; - } + g_return_if_fail(priv->info.type == SNDRV_CTL_ELEM_TYPE_INTEGER64); priv->info.value.integer.min = (long long)data[0]; priv->info.value.integer.max = (long long)data[1]; @@ -333,12 +324,10 @@ void alsactl_elem_info_get_enum_data(ALSACtlElemInfo *self, g_return_if_fail(ALSACTL_IS_ELEM_INFO(self)); priv = alsactl_elem_info_get_instance_private(self); + g_return_if_fail(data != NULL); g_return_if_fail(error == NULL || *error == NULL); - if (priv->info.type != SNDRV_CTL_ELEM_TYPE_ENUMERATED) { - generate_error(error, ENXIO); - return; - } + g_return_if_fail(priv->info.type == SNDRV_CTL_ELEM_TYPE_ENUMERATED); *data = (const gchar **)priv->enum_data; } @@ -362,12 +351,10 @@ void alsactl_elem_info_set_enum_data(ALSACtlElemInfo *self, g_return_if_fail(ALSACTL_IS_ELEM_INFO(self)); priv = alsactl_elem_info_get_instance_private(self); + g_return_if_fail(data != NULL); g_return_if_fail(error == NULL || *error == NULL); - if (priv->info.type != SNDRV_CTL_ELEM_TYPE_ENUMERATED) { - generate_error(error, ENXIO); - return; - } + g_return_if_fail(priv->info.type == SNDRV_CTL_ELEM_TYPE_ENUMERATED); g_strfreev(priv->enum_data); diff --git a/src/ctl/elem-value.c b/src/ctl/elem-value.c index 8023124..f09ec9d 100644 --- a/src/ctl/elem-value.c +++ b/src/ctl/elem-value.c @@ -103,8 +103,10 @@ void alsactl_elem_value_set_bool(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + + value = &priv->value; value_count = MIN(value_count, G_N_ELEMENTS(value->value.integer.value)); for (i = 0; i < value_count; ++i) value->value.integer.value[i] = (long)values[i]; @@ -128,8 +130,11 @@ void alsactl_elem_value_get_bool(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + g_return_if_fail(value_count != NULL); + + value = &priv->value; *value_count = MIN(*value_count, G_N_ELEMENTS(value->value.integer.value)); for (i = 0; i < *value_count; ++i) (*values)[i] = (gboolean)value->value.integer.value[i]; @@ -152,8 +157,10 @@ void alsactl_elem_value_set_int(ALSACtlElemValue *self, const gint32 *values, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + + value = &priv->value; value_count = MIN(value_count, G_N_ELEMENTS(value->value.integer.value)); for (i = 0; i < value_count; ++i) value->value.integer.value[i] = (long)values[i]; @@ -177,8 +184,11 @@ void alsactl_elem_value_get_int(ALSACtlElemValue *self, gint32 *const *values, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + g_return_if_fail(value_count != NULL); + + value = &priv->value; *value_count = MIN(*value_count, G_N_ELEMENTS(value->value.integer.value)); for (i = 0; i < *value_count; ++i) (*values)[i] = (gint32)value->value.integer.value[i]; @@ -202,8 +212,10 @@ void alsactl_elem_value_set_enum(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + + value = &priv->value; value_count = MIN(value_count, G_N_ELEMENTS(value->value.enumerated.item)); for (i = 0; i < value_count; ++i) value->value.enumerated.item[i] = (unsigned int)values[i]; @@ -227,8 +239,11 @@ void alsactl_elem_value_get_enum(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + g_return_if_fail(value_count != NULL); + + value = &priv->value; *value_count = MIN(*value_count, G_N_ELEMENTS(value->value.enumerated.item)); for (i = 0; i < *value_count; ++i) (*values)[i] = (guint32)value->value.enumerated.item[i]; @@ -251,8 +266,10 @@ void alsactl_elem_value_set_bytes(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + + value = &priv->value; value_count = MIN(value_count, G_N_ELEMENTS(value->value.bytes.data)); for (i = 0; i < value_count; ++i) value->value.bytes.data[i] = (long)values[i]; @@ -275,8 +292,11 @@ void alsactl_elem_value_get_bytes(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + g_return_if_fail(value_count != NULL); + + value = &priv->value; *value_count = MIN(*value_count, G_N_ELEMENTS(value->value.bytes.data)); for (i = 0; i < *value_count; ++i) (*values)[i] = (guint8)value->value.bytes.data[i]; @@ -300,8 +320,10 @@ void alsactl_elem_value_set_iec60958_channel_status(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(status != NULL); + + value = &priv->value; length = MIN(length, G_N_ELEMENTS(value->value.iec958.status)); for (i = 0; i < length; ++i) value->value.iec958.status[i] = status[i]; @@ -325,8 +347,11 @@ void alsactl_elem_value_get_iec60958_channel_status(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(status != NULL); + g_return_if_fail(length != NULL); + + value = &priv->value; *length = MIN(*length, G_N_ELEMENTS(value->value.iec958.status)); for (i = 0; i < *length; ++i) (*status)[i] = value->value.iec958.status[i]; @@ -350,8 +375,10 @@ void alsactl_elem_value_set_iec60958_user_data(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(data != NULL); + + value = &priv->value; length = MIN(length, G_N_ELEMENTS(value->value.iec958.subcode)); for (i = 0; i < length; ++i) value->value.iec958.subcode[i] = data[i]; @@ -375,8 +402,11 @@ void alsactl_elem_value_get_iec60958_user_data(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(data != NULL); + g_return_if_fail(length != NULL); + + value = &priv->value; *length = MIN(*length, G_N_ELEMENTS(value->value.iec958.subcode)); for (i = 0; i < *length; ++i) (*data)[i] = value->value.iec958.subcode[i]; @@ -399,8 +429,10 @@ void alsactl_elem_value_set_int64(ALSACtlElemValue *self, const gint64 *values, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + + value = &priv->value; value_count = MIN(value_count, G_N_ELEMENTS(value->value.integer64.value)); for (i = 0; i < value_count; ++i) value->value.integer64.value[i] = (long long)values[i]; @@ -424,8 +456,11 @@ void alsactl_elem_value_get_int64(ALSACtlElemValue *self, g_return_if_fail(ALSACTL_IS_ELEM_VALUE(self)); priv = alsactl_elem_value_get_instance_private(self); - value = &priv->value; + g_return_if_fail(values != NULL); + g_return_if_fail(value_count != NULL); + + value = &priv->value; *value_count = MIN(*value_count, G_N_ELEMENTS(value->value.integer64.value)); for (i = 0; i < *value_count; ++i) (*values)[i] = (gint64)value->value.integer64.value[i]; diff --git a/src/ctl/query.c b/src/ctl/query.c index 2d9db7c..cc8935b 100644 --- a/src/ctl/query.c +++ b/src/ctl/query.c @@ -123,10 +123,8 @@ void alsactl_get_card_id_list(guint **entries, gsize *entry_count, unsigned int count; unsigned int index; - if (entries == NULL || entry_count == NULL || error == NULL) { - generate_error(error, EINVAL); - return; - } + g_return_if_fail(entries != NULL); + g_return_if_fail(entry_count != NULL); g_return_if_fail(error == NULL || *error == NULL); prepare_udev_enum(&enumerator, error); @@ -168,7 +166,7 @@ void alsactl_get_card_id_list(guint **entries, gsize *entry_count, } } if (index != count) { - generate_error(error, ENOENT); + g_warn_if_reached(); g_free(*entries); *entries = NULL; goto end; -- 2.47.3