[PATCH 1/3] ASoC: adau17x1: Use empty struct initializer

[PATCH 1/3] ASoC: adau17x1: Use empty struct initializer

[Fabio Estevam]

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;
-- 
2.7.4

[PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Fabio Estevam]

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;
-- 
2.7.4

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Charles Keepax]

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

Applied "ASoC: wm9713: Use empty struct initializer" to the asoc tree

[Mark Brown]

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;
-- 
2.16.1

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Richard Fitzgerald]

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;
> 

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Fabio Estevam]

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.

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Fabio Estevam]

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 :-)

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Richard Fitzgerald]

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,

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Fabio Estevam]

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.

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Richard Fitzgerald]

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.

Re: [PATCH 2/3] ASoC: wm9713: Use empty struct initializer

[Fabio Estevam]

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.

[PATCH 3/3] ASoC: soc-dapm: Use empty struct initializer

[Fabio Estevam]

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)
-- 
2.7.4

Applied "ASoC: soc-dapm: Use empty struct initializer" to the asoc tree

[Mark Brown]

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)
-- 
2.16.1

Applied "ASoC: adau17x1: Use empty struct initializer" to the asoc tree

[Mark Brown]

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;
-- 
2.16.1