[alsa-devel] [PATCH 1/3] ASoC: adau17x1: Use empty struct initializer
From: Fabio Estevam fabio.estevam@nxp.com
{ 0 } only clears the first member of the structure.
The first member of the snd_soc_dapm_update struct is a pointer, and writing 0 to a pointer results in a sparse warning.
Use the empty struct initializer that clears all the struct members and fixes the sparse warning.
Signed-off-by: Fabio Estevam fabio.estevam@nxp.com --- sound/soc/codecs/adau17x1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index df34349..80c2a06 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -181,7 +181,7 @@ static int adau17x1_dsp_mux_enum_put(struct snd_kcontrol *kcontrol, struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); struct adau *adau = snd_soc_component_get_drvdata(component); struct soc_enum *e = (struct soc_enum *)kcontrol->private_value; - struct snd_soc_dapm_update update = { 0 }; + struct snd_soc_dapm_update update = {}; unsigned int stream = e->shift_l; unsigned int val, change; int reg;
From: Fabio Estevam fabio.estevam@nxp.com
{ 0 } only clears the first member of the structure.
The first member of the snd_soc_dapm_update struct is a pointer, and writing 0 to a pointer results in a sparse warning.
Use the empty struct initializer that clears all the struct members and fixes the sparse warning.
Cc: Charles Keepax ckeepax@opensource.cirrus.com Signed-off-by: Fabio Estevam fabio.estevam@nxp.com --- sound/soc/codecs/wm9713.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index 3d6cf00..643863b 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -235,7 +235,7 @@ static int wm9713_hp_mixer_put(struct snd_kcontrol *kcontrol, struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; unsigned int mixer, mask, shift, old; - struct snd_soc_dapm_update update = { 0 }; + struct snd_soc_dapm_update update = {}; bool change;
mixer = mc->shift >> 8;
On Wed, Feb 14, 2018 at 01:39:04PM -0200, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@nxp.com
{ 0 } only clears the first member of the structure.
The first member of the snd_soc_dapm_update struct is a pointer, and writing 0 to a pointer results in a sparse warning.
Use the empty struct initializer that clears all the struct members and fixes the sparse warning.
Cc: Charles Keepax ckeepax@opensource.cirrus.com Signed-off-by: Fabio Estevam fabio.estevam@nxp.com
Acked-by: Charles Keepax ckeepax@opensource.cirrus.com
Thanks, Charles
The patch
ASoC: wm9713: Use empty struct initializer
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 91cd00083d734258bf293dcf76793e2348be391a Mon Sep 17 00:00:00 2001
From: Fabio Estevam fabio.estevam@nxp.com Date: Wed, 14 Feb 2018 13:39:04 -0200 Subject: [PATCH] ASoC: wm9713: Use empty struct initializer
{ 0 } only clears the first member of the structure.
The first member of the snd_soc_dapm_update struct is a pointer, and writing 0 to a pointer results in a sparse warning.
Use the empty struct initializer that clears all the struct members and fixes the sparse warning.
Cc: Charles Keepax ckeepax@opensource.cirrus.com Signed-off-by: Fabio Estevam fabio.estevam@nxp.com Acked-by: Charles Keepax ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/wm9713.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index df7220656d98..093be043c1b0 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -235,7 +235,7 @@ static int wm9713_hp_mixer_put(struct snd_kcontrol *kcontrol, struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; unsigned int mixer, mask, shift, old; - struct snd_soc_dapm_update update = { 0 }; + struct snd_soc_dapm_update update = {}; bool change;
mixer = mc->shift >> 8;
On 14/02/18 15:39, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@nxp.com
{ 0 } only clears the first member of the structure.
Not according to the C99 and C11 standards. C11 6.7.9.21 (C99 has the same statement):
If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size than there are elements in the array, the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration.
C11 6.7.9.10 defines that "objects that have static storage duration" shall be initialized to zero.
So according to both C11 and C99 standards {0} should (and must) initialize the entire structure to zero.
The first member of the snd_soc_dapm_update struct is a pointer, and writing 0 to a pointer results in a sparse warning.
This is the actual problem you are trying to fix? The comment about { 0 } only clearing the first member is probably bogus, your actual problem is that it should have been { NULL } ?
So the fix works (because of 6.7.9.21 quoted above) but the commit message is incorrect/misleading about what the problem is and why this is a fix for it.
Use the empty struct initializer that clears all the struct members and fixes the sparse warning.
Cc: Charles Keepax ckeepax@opensource.cirrus.com Signed-off-by: Fabio Estevam fabio.estevam@nxp.com
sound/soc/codecs/wm9713.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c index 3d6cf00..643863b 100644 --- a/sound/soc/codecs/wm9713.c +++ b/sound/soc/codecs/wm9713.c @@ -235,7 +235,7 @@ static int wm9713_hp_mixer_put(struct snd_kcontrol *kcontrol, struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; unsigned int mixer, mask, shift, old;
- struct snd_soc_dapm_update update = { 0 };
struct snd_soc_dapm_update update = {}; bool change;
mixer = mc->shift >> 8;
On Wed, Feb 14, 2018 at 2:41 PM, Richard Fitzgerald rf@opensource.cirrus.com wrote:
This is the actual problem you are trying to fix? The comment about { 0 } only clearing the first member is probably bogus, your actual problem is that it should have been { NULL } ?
Changing to { NULL } fixes the sparse warning, but that would be a fragile fix because it relies on the order of the struct members.
If someday the struct changes in a way so that the first member is no longer a pointer, then we will have issues again.
Using {} is more error prone.
On Wed, Feb 14, 2018 at 2:49 PM, Fabio Estevam festevam@gmail.com wrote:
Using {} is more error prone.
I mean, less error prone :-)
On 14/02/18 16:49, Fabio Estevam wrote:
On Wed, Feb 14, 2018 at 2:41 PM, Richard Fitzgerald rf@opensource.cirrus.com wrote:
This is the actual problem you are trying to fix? The comment about { 0 } only clearing the first member is probably bogus, your actual problem is that it should have been { NULL } ?
Changing to { NULL } fixes the sparse warning, but that would be a fragile fix because it relies on the order of the struct members.
If someday the struct changes in a way so that the first member is no longer a pointer, then we will have issues again.
Using {} is more error prone.
I agree but your commit message didn't say that. It's irrelevant now Mark has merged the patch, but for anyone looking at the commit message later, the structure of your commit message implies that the problem is that {0} doesn't work correctly (probably untrue and certainly not the actual problem.) and your actual fix relies on precisely the behaviour that the first line of your commit message implies is broken.
I just like commit messages to be accurate about what the problem was and why. It's often said that an incorrect comment is worse than not having the comment at all, and the same could be said for commit messages,
On Wed, Feb 14, 2018 at 2:56 PM, Richard Fitzgerald rf@opensource.cirrus.com wrote:
I agree but your commit message didn't say that. It's irrelevant now Mark has merged the patch, but for anyone looking at the commit message later, the structure of your commit message implies that the problem is that {0} doesn't work correctly (probably untrue and certainly not
Yes, and that is the message I want to give:
{0} does not work correctly if the first member of the struct is a pointer, as we should not explicitly write 0 to pointers.
On 14/02/18 17:01, Fabio Estevam wrote:
On Wed, Feb 14, 2018 at 2:56 PM, Richard Fitzgerald rf@opensource.cirrus.com wrote:
I agree but your commit message didn't say that. It's irrelevant now Mark has merged the patch, but for anyone looking at the commit message later, the structure of your commit message implies that the problem is that {0} doesn't work correctly (probably untrue and certainly not
Yes, and that is the message I want to give:
{0} does not work correctly if the first member of the struct is a pointer, as we should not explicitly write 0 to pointers.
I gather that, so I'd have liked to your commit message to have said that. But what you actually said was
"{ 0 } only clears the first member of the structure."
which is a very different message.
On Wed, Feb 14, 2018 at 3:15 PM, Richard Fitzgerald rf@opensource.cirrus.com wrote:
I gather that, so I'd have liked to your commit message to have said that. But what you actually said was
"{ 0 } only clears the first member of the structure."
which is a very different message.
Got it. Maybe I should have written like this instead:
"{ 0 } only clears explicitly the first member of the structure."
Thanks for the feedback.
From: Fabio Estevam fabio.estevam@nxp.com
{ NULL } only clears the first member of the structure.
Even though the first member of the snd_soc_dapm_update struct is a pointer,it is more robust to use the empty struct initializer that clears all the struct members.
Signed-off-by: Fabio Estevam fabio.estevam@nxp.com --- sound/soc/soc-dapm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 92894d9..2f34590 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3165,7 +3165,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, unsigned int invert = mc->invert; unsigned int val, rval = 0; int connect, rconnect = -1, change, reg_change = 0; - struct snd_soc_dapm_update update = { NULL }; + struct snd_soc_dapm_update update = {}; int ret = 0;
val = (ucontrol->value.integer.value[0] & mask); @@ -3292,7 +3292,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, unsigned int *item = ucontrol->value.enumerated.item; unsigned int val, change, reg_change = 0; unsigned int mask; - struct snd_soc_dapm_update update = { NULL }; + struct snd_soc_dapm_update update = {}; int ret = 0;
if (item[0] >= e->items)
The patch
ASoC: soc-dapm: Use empty struct initializer
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 33d9245c6092336420c67f761b3bdcdd6c7c1d5d Mon Sep 17 00:00:00 2001
From: Fabio Estevam fabio.estevam@nxp.com Date: Wed, 14 Feb 2018 13:39:05 -0200 Subject: [PATCH] ASoC: soc-dapm: Use empty struct initializer
{ NULL } only clears the first member of the structure.
Even though the first member of the snd_soc_dapm_update struct is a pointer,it is more robust to use the empty struct initializer that clears all the struct members.
Signed-off-by: Fabio Estevam fabio.estevam@nxp.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-dapm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 92894d9cac19..2f34590bcb72 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3165,7 +3165,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol, unsigned int invert = mc->invert; unsigned int val, rval = 0; int connect, rconnect = -1, change, reg_change = 0; - struct snd_soc_dapm_update update = { NULL }; + struct snd_soc_dapm_update update = {}; int ret = 0;
val = (ucontrol->value.integer.value[0] & mask); @@ -3292,7 +3292,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol, unsigned int *item = ucontrol->value.enumerated.item; unsigned int val, change, reg_change = 0; unsigned int mask; - struct snd_soc_dapm_update update = { NULL }; + struct snd_soc_dapm_update update = {}; int ret = 0;
if (item[0] >= e->items)
The patch
ASoC: adau17x1: Use empty struct initializer
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From c90ef8c7419180ee873dd64e5ae162d32faca762 Mon Sep 17 00:00:00 2001
From: Fabio Estevam fabio.estevam@nxp.com Date: Wed, 14 Feb 2018 13:39:03 -0200 Subject: [PATCH] ASoC: adau17x1: Use empty struct initializer
{ 0 } only clears the first member of the structure.
The first member of the snd_soc_dapm_update struct is a pointer, and writing 0 to a pointer results in a sparse warning.
Use the empty struct initializer that clears all the struct members and fixes the sparse warning.
Signed-off-by: Fabio Estevam fabio.estevam@nxp.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/adau17x1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index df3434982b43..80c2a06285bb 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -181,7 +181,7 @@ static int adau17x1_dsp_mux_enum_put(struct snd_kcontrol *kcontrol, struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); struct adau *adau = snd_soc_component_get_drvdata(component); struct soc_enum *e = (struct soc_enum *)kcontrol->private_value; - struct snd_soc_dapm_update update = { 0 }; + struct snd_soc_dapm_update update = {}; unsigned int stream = e->shift_l; unsigned int val, change; int reg;
participants (4)
-
Charles Keepax -
Fabio Estevam -
Mark Brown -
Richard Fitzgerald