[alsa-devel] [PATCH 1/1] ASoC: sti: fix compilation warnings
Add check on of_property_read_string return to avoid compilation warning.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com --- sound/soc/sti/uniperif_player.c | 32 +++++++++++++++++++------------- sound/soc/sti/uniperif_reader.c | 6 +++++- 2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c index f6eefe1..d90a3fb 100644 --- a/sound/soc/sti/uniperif_player.c +++ b/sound/soc/sti/uniperif_player.c @@ -989,8 +989,8 @@ static int uni_player_parse_dt(struct platform_device *pdev, if (!info) return -ENOMEM;
- of_property_read_u32(pnode, "version", &player->ver); - if (player->ver == SND_ST_UNIPERIF_VERSION_UNKNOWN) { + if ((of_property_read_u32(pnode, "version", &player->ver)) || + (player->ver == SND_ST_UNIPERIF_VERSION_UNKNOWN)) { dev_err(dev, "Unknown uniperipheral version "); return -EINVAL; } @@ -998,19 +998,25 @@ static int uni_player_parse_dt(struct platform_device *pdev, if (player->ver >= SND_ST_UNIPERIF_VERSION_UNI_PLR_TOP_1_0) info->underflow_enabled = 1;
- of_property_read_u32(pnode, "uniperiph-id", &info->id); + if (of_property_read_u32(pnode, "uniperiph-id", &info->id)) { + dev_err(dev, "uniperipheral id not defined"); + return -EINVAL; + }
/* Read the device mode property */ - of_property_read_string(pnode, "mode", &mode); - - if (strcasecmp(mode, "hdmi") == 0) - info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_HDMI; - else if (strcasecmp(mode, "pcm") == 0) - info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_PCM; - else if (strcasecmp(mode, "spdif") == 0) - info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_SPDIF; - else - info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_NONE; + if (!of_property_read_string(pnode, "mode", &mode)) { + if (strcasecmp(mode, "hdmi") == 0) + info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_HDMI; + else if (strcasecmp(mode, "pcm") == 0) + info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_PCM; + else if (strcasecmp(mode, "spdif") == 0) + info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_SPDIF; + else + info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_NONE; + } else { + dev_err(dev, "uniperipheral mode not defined"); + return -EINVAL; + }
/* Save the info structure */ player->info = info; diff --git a/sound/soc/sti/uniperif_reader.c b/sound/soc/sti/uniperif_reader.c index c502626..7ddbd14 100644 --- a/sound/soc/sti/uniperif_reader.c +++ b/sound/soc/sti/uniperif_reader.c @@ -316,7 +316,11 @@ static int uni_reader_parse_dt(struct platform_device *pdev, if (!info) return -ENOMEM;
- of_property_read_u32(node, "version", &reader->ver); + if ((of_property_read_u32(node, "version", &reader->ver)) || + (reader->ver == SND_ST_UNIPERIF_VERSION_UNKNOWN)) { + dev_err(&pdev->dev, "Unknown uniperipheral version "); + return -EINVAL; + }
/* Save the info structure */ reader->info = info;
On Wed, 09 Sep 2015 19:04:46 +0200, Arnaud Pouliquen wrote:
Add check on of_property_read_string return to avoid compilation warning.
The purpose of the patch isn't to avoid compilation warning. It's for fixing the bugs in the code. Fixing compile warning is just a consequence, not the goal.
Please rewrite the subject and the change log text appropriately.
Some review comments below:
- of_property_read_u32(pnode, "version", &player->ver);
- if (player->ver == SND_ST_UNIPERIF_VERSION_UNKNOWN) {
- if ((of_property_read_u32(pnode, "version", &player->ver)) ||
Better to drop superfluous parentheses.
(player->ver == SND_ST_UNIPERIF_VERSION_UNKNOWN)) {
In general, such a sanity check should be rather checking the valid value ranges, something like: if (ver < VER_MIN || ver > VER_MAX) error(); In short, you can't trust the value passed by DT, so you need to check it strictly (if needed).
But this doesn't have to be covered by this patch. Better to be in another fix patch.
@@ -998,19 +998,25 @@ static int uni_player_parse_dt(struct platform_device *pdev, if (player->ver >= SND_ST_UNIPERIF_VERSION_UNI_PLR_TOP_1_0) info->underflow_enabled = 1;
- of_property_read_u32(pnode, "uniperiph-id", &info->id);
if (of_property_read_u32(pnode, "uniperiph-id", &info->id)) {
dev_err(dev, "uniperipheral id not defined");return -EINVAL;}
/* Read the device mode property */
- of_property_read_string(pnode, "mode", &mode);
- if (strcasecmp(mode, "hdmi") == 0)
info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_HDMI;- else if (strcasecmp(mode, "pcm") == 0)
info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_PCM;- else if (strcasecmp(mode, "spdif") == 0)
info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_SPDIF;- else
info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_NONE;
- if (!of_property_read_string(pnode, "mode", &mode)) {
if (strcasecmp(mode, "hdmi") == 0)info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_HDMI;else if (strcasecmp(mode, "pcm") == 0)info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_PCM;else if (strcasecmp(mode, "spdif") == 0)info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_SPDIF;elseinfo->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_NONE;- } else {
dev_err(dev, "uniperipheral mode not defined");return -EINVAL;- }
A form like below would be clearer:
if (of_property_read_string(pnode, "mode", &mode)) { dev_err(dev, "uniperipheral mode not defined"); return -EINVAL; }
if (strcasecmp(mode, "hdmi") == 0) info->player_type = SND_ST_UNIPERIF_PLAYER_TYPE_HDMI; ....
thanks,
Takashi
participants (2)
-
Arnaud Pouliquen -
Takashi Iwai