[alsa-devel] [PATCH v2 1/3] ALSA: ice1712: remove unneeded return statement
the functions: snd_ice1712_akm4xxx_build_controls snd_ice1712_build_pro_mixer snd_ctl_add snd_ak4114_build prodigy192_ak4114_init snd_ak4113_build are all returning either 0 or a negetive error value. so we can easily remove the check for a negative value and return the value instead.
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org ---
change in v2: returning the return value of the function directly.
sound/pci/ice1712/hoontech.c | 6 +----- sound/pci/ice1712/ice1712.c | 19 ++++++------------- sound/pci/ice1712/ice1724.c | 7 ++----- sound/pci/ice1712/juli.c | 5 +---- sound/pci/ice1712/prodigy192.c | 4 +--- sound/pci/ice1712/quartet.c | 5 +---- 6 files changed, 12 insertions(+), 34 deletions(-)
diff --git a/sound/pci/ice1712/hoontech.c b/sound/pci/ice1712/hoontech.c index 59e37c5..a40001c 100644 --- a/sound/pci/ice1712/hoontech.c +++ b/sound/pci/ice1712/hoontech.c @@ -309,11 +309,7 @@ static int snd_ice1712_value_init(struct snd_ice1712 *ice) return err;
/* ak4524 controls */ - err = snd_ice1712_akm4xxx_build_controls(ice); - if (err < 0) - return err; - - return 0; + return snd_ice1712_akm4xxx_build_controls(ice); }
static int snd_ice1712_ez8_init(struct snd_ice1712 *ice) diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c index 48a0c33..5975334 100644 --- a/sound/pci/ice1712/ice1712.c +++ b/sound/pci/ice1712/ice1712.c @@ -1295,10 +1295,7 @@ static int snd_ice1712_pcm_profi(struct snd_ice1712 *ice, int device, struct snd return err; }
- err = snd_ice1712_build_pro_mixer(ice); - if (err < 0) - return err; - return 0; + return snd_ice1712_build_pro_mixer(ice); }
/* @@ -1545,10 +1542,9 @@ static int snd_ice1712_ac97_mixer(struct snd_ice1712 *ice) dev_warn(ice->card->dev, "cannot initialize ac97 for consumer, skipped\n"); else { - err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_ice1712_mixer_digmix_route_ac97, ice)); - if (err < 0) - return err; - return 0; + return snd_ctl_add(ice->card, + snd_ctl_new1(&snd_ice1712_mixer_digmix_route_ac97, + ice)); } }
@@ -2497,11 +2493,8 @@ static int snd_ice1712_build_controls(struct snd_ice1712 *ice) err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_ice1712_mixer_pro_volume_rate, ice)); if (err < 0) return err; - err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_ice1712_mixer_pro_peak, ice)); - if (err < 0) - return err; - - return 0; + return snd_ctl_add(ice->card, + snd_ctl_new1(&snd_ice1712_mixer_pro_peak, ice)); }
static int snd_ice1712_free(struct snd_ice1712 *ice) diff --git a/sound/pci/ice1712/ice1724.c b/sound/pci/ice1712/ice1724.c index f633e3b..ea53167 100644 --- a/sound/pci/ice1712/ice1724.c +++ b/sound/pci/ice1712/ice1724.c @@ -2497,11 +2497,8 @@ static int snd_vt1724_build_controls(struct snd_ice1712 *ice) return err; }
- err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_vt1724_mixer_pro_peak, ice)); - if (err < 0) - return err; - - return 0; + return snd_ctl_add(ice->card, + snd_ctl_new1(&snd_vt1724_mixer_pro_peak, ice)); }
static int snd_vt1724_free(struct snd_ice1712 *ice) diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c index 7a6c078..a1536c1 100644 --- a/sound/pci/ice1712/juli.c +++ b/sound/pci/ice1712/juli.c @@ -475,11 +475,8 @@ static int juli_add_controls(struct snd_ice1712 *ice) return err;
/* only capture SPDIF over AK4114 */ - err = snd_ak4114_build(spec->ak4114, NULL, + return snd_ak4114_build(spec->ak4114, NULL, ice->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream); - if (err < 0) - return err; - return 0; }
/* diff --git a/sound/pci/ice1712/prodigy192.c b/sound/pci/ice1712/prodigy192.c index 1eb151aa..3919aed 100644 --- a/sound/pci/ice1712/prodigy192.c +++ b/sound/pci/ice1712/prodigy192.c @@ -758,10 +758,8 @@ static int prodigy192_init(struct snd_ice1712 *ice) "AK4114 initialized with status %d\n", err); } else dev_dbg(ice->card->dev, "AK4114 not found\n"); - if (err < 0) - return err;
- return 0; + return err; }
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c index d4caf9d..6f55e02 100644 --- a/sound/pci/ice1712/quartet.c +++ b/sound/pci/ice1712/quartet.c @@ -833,11 +833,8 @@ static int qtet_add_controls(struct snd_ice1712 *ice) if (err < 0) return err; /* only capture SPDIF over AK4113 */ - err = snd_ak4113_build(spec->ak4113, + return snd_ak4113_build(spec->ak4113, ice->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream); - if (err < 0) - return err; - return 0; }
static inline int qtet_is_spdif_master(struct snd_ice1712 *ice)
earlier we were ignoring the return value of snd_ak4114_create and always returning 0. now we are returning the actual status. revo_init is calling this function, and revo_init is checking the return value.
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org --- sound/pci/ice1712/revo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/ice1712/revo.c b/sound/pci/ice1712/revo.c index 1112ec1..2835196 100644 --- a/sound/pci/ice1712/revo.c +++ b/sound/pci/ice1712/revo.c @@ -498,7 +498,7 @@ static int ap192_ak4114_init(struct snd_ice1712 *ice) * No reason to stop capture stream due to incorrect checks */ spec->ak4114->check_flags = AK4114_CHECK_NO_RATE;
- return 0; /* error ignored; it's no fatal error */ + return err; }
static int revo_init(struct snd_ice1712 *ice)
At Fri, 14 Nov 2014 16:09:06 +0530, Sudip Mukherjee wrote:
earlier we were ignoring the return value of snd_ak4114_create and always returning 0. now we are returning the actual status. revo_init is calling this function, and revo_init is checking the return value.
It's not enough only to change there. The problem is that spec->ak4114 is dereferenced after snd_ak4114_create(), and this might be NULL. So it should return the error before that point.
Takashi
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org
sound/pci/ice1712/revo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/ice1712/revo.c b/sound/pci/ice1712/revo.c index 1112ec1..2835196 100644 --- a/sound/pci/ice1712/revo.c +++ b/sound/pci/ice1712/revo.c @@ -498,7 +498,7 @@ static int ap192_ak4114_init(struct snd_ice1712 *ice) * No reason to stop capture stream due to incorrect checks */ spec->ak4114->check_flags = AK4114_CHECK_NO_RATE;
- return 0; /* error ignored; it's no fatal error */
- return err;
}
static int revo_init(struct snd_ice1712 *ice)
1.8.1.2
On Fri, Nov 14, 2014 at 11:43:31AM +0100, Takashi Iwai wrote:
At Fri, 14 Nov 2014 16:09:06 +0530, Sudip Mukherjee wrote:
earlier we were ignoring the return value of snd_ak4114_create and always returning 0. now we are returning the actual status. revo_init is calling this function, and revo_init is checking the return value.
It's not enough only to change there. The problem is that spec->ak4114 is dereferenced after snd_ak4114_create(), and this might be NULL. So it should return the error before that point.
oops .. i should have looked carefully.
one more thought: shouldn't we also do a kfree(spec) on error ?
thanks sudip
Takashi
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org
sound/pci/ice1712/revo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/ice1712/revo.c b/sound/pci/ice1712/revo.c index 1112ec1..2835196 100644 --- a/sound/pci/ice1712/revo.c +++ b/sound/pci/ice1712/revo.c @@ -498,7 +498,7 @@ static int ap192_ak4114_init(struct snd_ice1712 *ice) * No reason to stop capture stream due to incorrect checks */ spec->ak4114->check_flags = AK4114_CHECK_NO_RATE;
- return 0; /* error ignored; it's no fatal error */
- return err;
}
static int revo_init(struct snd_ice1712 *ice)
1.8.1.2
At Fri, 14 Nov 2014 16:29:15 +0530, Sudip Mukherjee wrote:
On Fri, Nov 14, 2014 at 11:43:31AM +0100, Takashi Iwai wrote:
At Fri, 14 Nov 2014 16:09:06 +0530, Sudip Mukherjee wrote:
earlier we were ignoring the return value of snd_ak4114_create and always returning 0. now we are returning the actual status. revo_init is calling this function, and revo_init is checking the return value.
It's not enough only to change there. The problem is that spec->ak4114 is dereferenced after snd_ak4114_create(), and this might be NULL. So it should return the error before that point.
oops .. i should have looked carefully.
one more thought: shouldn't we also do a kfree(spec) on error ?
Not necessary. It's freed in the card's destructor.
Takashi
thanks sudip
Takashi
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org
sound/pci/ice1712/revo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/ice1712/revo.c b/sound/pci/ice1712/revo.c index 1112ec1..2835196 100644 --- a/sound/pci/ice1712/revo.c +++ b/sound/pci/ice1712/revo.c @@ -498,7 +498,7 @@ static int ap192_ak4114_init(struct snd_ice1712 *ice) * No reason to stop capture stream due to incorrect checks */ spec->ak4114->check_flags = AK4114_CHECK_NO_RATE;
- return 0; /* error ignored; it's no fatal error */
- return err;
}
static int revo_init(struct snd_ice1712 *ice)
1.8.1.2
buf_size was initialized with snd_pcm_lib_buffer_bytes, but never used. and so it is safe to be deleted.
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org --- sound/pci/ice1712/ice1712.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c index 5975334..6525191 100644 --- a/sound/pci/ice1712/ice1712.c +++ b/sound/pci/ice1712/ice1712.c @@ -620,10 +620,9 @@ static int snd_ice1712_playback_ds_prepare(struct snd_pcm_substream *substream) { struct snd_ice1712 *ice = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; - u32 period_size, buf_size, rate, tmp, chn; + u32 period_size, rate, tmp, chn;
period_size = snd_pcm_lib_period_bytes(substream) - 1; - buf_size = snd_pcm_lib_buffer_bytes(substream) - 1; tmp = 0x0064; if (snd_pcm_format_width(runtime->format) == 16) tmp &= ~0x04;
At Fri, 14 Nov 2014 16:09:07 +0530, Sudip Mukherjee wrote:
buf_size was initialized with snd_pcm_lib_buffer_bytes, but never used. and so it is safe to be deleted.
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org
Thanks, applied.
Takashi
sound/pci/ice1712/ice1712.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c index 5975334..6525191 100644 --- a/sound/pci/ice1712/ice1712.c +++ b/sound/pci/ice1712/ice1712.c @@ -620,10 +620,9 @@ static int snd_ice1712_playback_ds_prepare(struct snd_pcm_substream *substream) { struct snd_ice1712 *ice = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime;
- u32 period_size, buf_size, rate, tmp, chn;
u32 period_size, rate, tmp, chn;
period_size = snd_pcm_lib_period_bytes(substream) - 1;
- buf_size = snd_pcm_lib_buffer_bytes(substream) - 1; tmp = 0x0064; if (snd_pcm_format_width(runtime->format) == 16) tmp &= ~0x04;
-- 1.8.1.2
At Fri, 14 Nov 2014 16:09:05 +0530, Sudip Mukherjee wrote:
the functions: snd_ice1712_akm4xxx_build_controls snd_ice1712_build_pro_mixer snd_ctl_add snd_ak4114_build prodigy192_ak4114_init snd_ak4113_build are all returning either 0 or a negetive error value. so we can easily remove the check for a negative value and return the value instead.
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org
change in v2: returning the return value of the function directly.
Thanks, applied this one.
Takashi
sound/pci/ice1712/hoontech.c | 6 +----- sound/pci/ice1712/ice1712.c | 19 ++++++------------- sound/pci/ice1712/ice1724.c | 7 ++----- sound/pci/ice1712/juli.c | 5 +---- sound/pci/ice1712/prodigy192.c | 4 +--- sound/pci/ice1712/quartet.c | 5 +---- 6 files changed, 12 insertions(+), 34 deletions(-)
diff --git a/sound/pci/ice1712/hoontech.c b/sound/pci/ice1712/hoontech.c index 59e37c5..a40001c 100644 --- a/sound/pci/ice1712/hoontech.c +++ b/sound/pci/ice1712/hoontech.c @@ -309,11 +309,7 @@ static int snd_ice1712_value_init(struct snd_ice1712 *ice) return err;
/* ak4524 controls */
- err = snd_ice1712_akm4xxx_build_controls(ice);
- if (err < 0)
return err;- return 0;
- return snd_ice1712_akm4xxx_build_controls(ice);
}
static int snd_ice1712_ez8_init(struct snd_ice1712 *ice) diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c index 48a0c33..5975334 100644 --- a/sound/pci/ice1712/ice1712.c +++ b/sound/pci/ice1712/ice1712.c @@ -1295,10 +1295,7 @@ static int snd_ice1712_pcm_profi(struct snd_ice1712 *ice, int device, struct snd return err; }
- err = snd_ice1712_build_pro_mixer(ice);
- if (err < 0)
return err;- return 0;
- return snd_ice1712_build_pro_mixer(ice);
}
/* @@ -1545,10 +1542,9 @@ static int snd_ice1712_ac97_mixer(struct snd_ice1712 *ice) dev_warn(ice->card->dev, "cannot initialize ac97 for consumer, skipped\n"); else {
err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_ice1712_mixer_digmix_route_ac97, ice));if (err < 0)return err;return 0;
return snd_ctl_add(ice->card,snd_ctl_new1(&snd_ice1712_mixer_digmix_route_ac97,ice));@@ -2497,11 +2493,8 @@ static int snd_ice1712_build_controls(struct snd_ice1712 *ice) err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_ice1712_mixer_pro_volume_rate, ice)); if (err < 0) return err;
- err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_ice1712_mixer_pro_peak, ice));
- if (err < 0)
return err;- return 0;
- return snd_ctl_add(ice->card,
snd_ctl_new1(&snd_ice1712_mixer_pro_peak, ice));}
static int snd_ice1712_free(struct snd_ice1712 *ice) diff --git a/sound/pci/ice1712/ice1724.c b/sound/pci/ice1712/ice1724.c index f633e3b..ea53167 100644 --- a/sound/pci/ice1712/ice1724.c +++ b/sound/pci/ice1712/ice1724.c @@ -2497,11 +2497,8 @@ static int snd_vt1724_build_controls(struct snd_ice1712 *ice) return err; }
- err = snd_ctl_add(ice->card, snd_ctl_new1(&snd_vt1724_mixer_pro_peak, ice));
- if (err < 0)
return err;- return 0;
- return snd_ctl_add(ice->card,
snd_ctl_new1(&snd_vt1724_mixer_pro_peak, ice));}
static int snd_vt1724_free(struct snd_ice1712 *ice) diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c index 7a6c078..a1536c1 100644 --- a/sound/pci/ice1712/juli.c +++ b/sound/pci/ice1712/juli.c @@ -475,11 +475,8 @@ static int juli_add_controls(struct snd_ice1712 *ice) return err;
/* only capture SPDIF over AK4114 */
- err = snd_ak4114_build(spec->ak4114, NULL,
- return snd_ak4114_build(spec->ak4114, NULL, ice->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream);
- if (err < 0)
return err;- return 0;
}
/* diff --git a/sound/pci/ice1712/prodigy192.c b/sound/pci/ice1712/prodigy192.c index 1eb151aa..3919aed 100644 --- a/sound/pci/ice1712/prodigy192.c +++ b/sound/pci/ice1712/prodigy192.c @@ -758,10 +758,8 @@ static int prodigy192_init(struct snd_ice1712 *ice) "AK4114 initialized with status %d\n", err); } else dev_dbg(ice->card->dev, "AK4114 not found\n");
if (err < 0)
return err;return 0;
- return err;
}
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c index d4caf9d..6f55e02 100644 --- a/sound/pci/ice1712/quartet.c +++ b/sound/pci/ice1712/quartet.c @@ -833,11 +833,8 @@ static int qtet_add_controls(struct snd_ice1712 *ice) if (err < 0) return err; /* only capture SPDIF over AK4113 */
- err = snd_ak4113_build(spec->ak4113,
- return snd_ak4113_build(spec->ak4113, ice->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream);
- if (err < 0)
return err;- return 0;
}
static inline int qtet_is_spdif_master(struct snd_ice1712 *ice)
1.8.1.2
participants (2)
-
Sudip Mukherjee -
Takashi Iwai