[PATCH] ALSA: pcm: Mostly constify constraints

[PATCH] ALSA: pcm: Mostly constify constraints

[Mark Brown]

There is no need for the constraints to be modified while being applied so
they can be declared const, allowing drivers to declare constraint lists
const if they wish.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/sound/pcm.h     |    6 +++---
 sound/core/pcm_lib.c    |   10 +++++-----
 sound/core/pcm_native.c |    4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index e7afcc9..3b8f6ca 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -217,7 +217,7 @@ struct snd_pcm_hw_rule {
 	snd_pcm_hw_rule_func_t func;
 	int var;
 	int deps[4];
-	void *private;
+	const void *private;
 };
 
 struct snd_pcm_hw_constraints {
@@ -810,7 +810,7 @@ int snd_pcm_hw_constraint_integer(struct snd_pcm_runtime *runtime, snd_pcm_hw_pa
 int snd_pcm_hw_constraint_list(struct snd_pcm_runtime *runtime, 
 			       unsigned int cond,
 			       snd_pcm_hw_param_t var,
-			       struct snd_pcm_hw_constraint_list *l);
+			       const struct snd_pcm_hw_constraint_list *l);
 int snd_pcm_hw_constraint_ratnums(struct snd_pcm_runtime *runtime, 
 				  unsigned int cond,
 				  snd_pcm_hw_param_t var,
@@ -835,7 +835,7 @@ int snd_pcm_hw_rule_noresample(struct snd_pcm_runtime *runtime,
 int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime,
 			unsigned int cond,
 			int var,
-			snd_pcm_hw_rule_func_t func, void *private,
+			snd_pcm_hw_rule_func_t func, const void *private,
 			int dep, ...);
 
 int snd_pcm_format_signed(snd_pcm_format_t format);
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 8f312fa6..4372de5 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1104,7 +1104,7 @@ static int snd_interval_step(struct snd_interval *i, unsigned int min, unsigned
  */
 int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime, unsigned int cond,
 			int var,
-			snd_pcm_hw_rule_func_t func, void *private,
+			snd_pcm_hw_rule_func_t func, const void *private,
 			int dep, ...)
 {
 	struct snd_pcm_hw_constraints *constrs = &runtime->hw_constraints;
@@ -1233,7 +1233,7 @@ EXPORT_SYMBOL(snd_pcm_hw_constraint_minmax);
 static int snd_pcm_hw_rule_list(struct snd_pcm_hw_params *params,
 				struct snd_pcm_hw_rule *rule)
 {
-	struct snd_pcm_hw_constraint_list *list = rule->private;
+	const struct snd_pcm_hw_constraint_list *list = rule->private;
 	return snd_interval_list(hw_param_interval(params, rule->var), list->count, list->list, list->mask);
 }		
 
@@ -1250,7 +1250,7 @@ static int snd_pcm_hw_rule_list(struct snd_pcm_hw_params *params,
 int snd_pcm_hw_constraint_list(struct snd_pcm_runtime *runtime,
 			       unsigned int cond,
 			       snd_pcm_hw_param_t var,
-			       struct snd_pcm_hw_constraint_list *l)
+			       const struct snd_pcm_hw_constraint_list *l)
 {
 	return snd_pcm_hw_rule_add(runtime, cond, var,
 				   snd_pcm_hw_rule_list, l,
@@ -1262,7 +1262,7 @@ EXPORT_SYMBOL(snd_pcm_hw_constraint_list);
 static int snd_pcm_hw_rule_ratnums(struct snd_pcm_hw_params *params,
 				   struct snd_pcm_hw_rule *rule)
 {
-	struct snd_pcm_hw_constraint_ratnums *r = rule->private;
+	const struct snd_pcm_hw_constraint_ratnums *r = rule->private;
 	unsigned int num = 0, den = 0;
 	int err;
 	err = snd_interval_ratnum(hw_param_interval(params, rule->var),
@@ -1296,7 +1296,7 @@ EXPORT_SYMBOL(snd_pcm_hw_constraint_ratnums);
 static int snd_pcm_hw_rule_ratdens(struct snd_pcm_hw_params *params,
 				   struct snd_pcm_hw_rule *rule)
 {
-	struct snd_pcm_hw_constraint_ratdens *r = rule->private;
+	const struct snd_pcm_hw_constraint_ratdens *r = rule->private;
 	unsigned int num = 0, den = 0;
 	int err = snd_interval_ratden(hw_param_interval(params, rule->var),
 				  r->nrats, r->rats, &num, &den);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 53b5ada..5cc8c4f 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1775,7 +1775,7 @@ const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
 static int snd_pcm_hw_rule_rate(struct snd_pcm_hw_params *params,
 				struct snd_pcm_hw_rule *rule)
 {
-	struct snd_pcm_hardware *hw = rule->private;
+	const struct snd_pcm_hardware *hw = rule->private;
 	return snd_interval_list(hw_param_interval(params, rule->var),
 				 snd_pcm_known_rates.count,
 				 snd_pcm_known_rates.list, hw->rates);
@@ -1785,7 +1785,7 @@ static int snd_pcm_hw_rule_buffer_bytes_max(struct snd_pcm_hw_params *params,
 					    struct snd_pcm_hw_rule *rule)
 {
 	struct snd_interval t;
-	struct snd_pcm_substream *substream = rule->private;
+	const struct snd_pcm_substream *substream = rule->private;
 	t.min = 0;
 	t.max = substream->buffer_bytes_max;
 	t.openmin = 0;
-- 
1.7.10

Re: [PATCH] ALSA: pcm: Mostly constify constraints

[Takashi Iwai]

At Wed,  4 Jul 2012 18:35:12 +0100,
Mark Brown wrote:
> 
> There is no need for the constraints to be modified while being
> applied

Many drivers pass the local instance to rule->private, and it's not
always guaranteed to be const.  For example, you can imagine some
state modified kept in the struct while it's modified via rules.

In short: you'd have seen many compile warnings for non-ASoC drivers
if you run make once with this patch ;)

As a safe side fix, how about just adding const to
snd_pcm_hw_constraint_list() & co, and expclitly cast to non-const
later?


Takashi

> so
> they can be declared const, allowing drivers to declare constraint lists
> const if they wish.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  include/sound/pcm.h     |    6 +++---
>  sound/core/pcm_lib.c    |   10 +++++-----
>  sound/core/pcm_native.c |    4 ++--
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index e7afcc9..3b8f6ca 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -217,7 +217,7 @@ struct snd_pcm_hw_rule {
>  	snd_pcm_hw_rule_func_t func;
>  	int var;
>  	int deps[4];
> -	void *private;
> +	const void *private;
>  };
>  
>  struct snd_pcm_hw_constraints {
> @@ -810,7 +810,7 @@ int snd_pcm_hw_constraint_integer(struct snd_pcm_runtime *runtime, snd_pcm_hw_pa
>  int snd_pcm_hw_constraint_list(struct snd_pcm_runtime *runtime, 
>  			       unsigned int cond,
>  			       snd_pcm_hw_param_t var,
> -			       struct snd_pcm_hw_constraint_list *l);
> +			       const struct snd_pcm_hw_constraint_list *l);
>  int snd_pcm_hw_constraint_ratnums(struct snd_pcm_runtime *runtime, 
>  				  unsigned int cond,
>  				  snd_pcm_hw_param_t var,
> @@ -835,7 +835,7 @@ int snd_pcm_hw_rule_noresample(struct snd_pcm_runtime *runtime,
>  int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime,
>  			unsigned int cond,
>  			int var,
> -			snd_pcm_hw_rule_func_t func, void *private,
> +			snd_pcm_hw_rule_func_t func, const void *private,
>  			int dep, ...);
>  
>  int snd_pcm_format_signed(snd_pcm_format_t format);
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index 8f312fa6..4372de5 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -1104,7 +1104,7 @@ static int snd_interval_step(struct snd_interval *i, unsigned int min, unsigned
>   */
>  int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime, unsigned int cond,
>  			int var,
> -			snd_pcm_hw_rule_func_t func, void *private,
> +			snd_pcm_hw_rule_func_t func, const void *private,
>  			int dep, ...)
>  {
>  	struct snd_pcm_hw_constraints *constrs = &runtime->hw_constraints;
> @@ -1233,7 +1233,7 @@ EXPORT_SYMBOL(snd_pcm_hw_constraint_minmax);
>  static int snd_pcm_hw_rule_list(struct snd_pcm_hw_params *params,
>  				struct snd_pcm_hw_rule *rule)
>  {
> -	struct snd_pcm_hw_constraint_list *list = rule->private;
> +	const struct snd_pcm_hw_constraint_list *list = rule->private;
>  	return snd_interval_list(hw_param_interval(params, rule->var), list->count, list->list, list->mask);
>  }		
>  
> @@ -1250,7 +1250,7 @@ static int snd_pcm_hw_rule_list(struct snd_pcm_hw_params *params,
>  int snd_pcm_hw_constraint_list(struct snd_pcm_runtime *runtime,
>  			       unsigned int cond,
>  			       snd_pcm_hw_param_t var,
> -			       struct snd_pcm_hw_constraint_list *l)
> +			       const struct snd_pcm_hw_constraint_list *l)
>  {
>  	return snd_pcm_hw_rule_add(runtime, cond, var,
>  				   snd_pcm_hw_rule_list, l,
> @@ -1262,7 +1262,7 @@ EXPORT_SYMBOL(snd_pcm_hw_constraint_list);
>  static int snd_pcm_hw_rule_ratnums(struct snd_pcm_hw_params *params,
>  				   struct snd_pcm_hw_rule *rule)
>  {
> -	struct snd_pcm_hw_constraint_ratnums *r = rule->private;
> +	const struct snd_pcm_hw_constraint_ratnums *r = rule->private;
>  	unsigned int num = 0, den = 0;
>  	int err;
>  	err = snd_interval_ratnum(hw_param_interval(params, rule->var),
> @@ -1296,7 +1296,7 @@ EXPORT_SYMBOL(snd_pcm_hw_constraint_ratnums);
>  static int snd_pcm_hw_rule_ratdens(struct snd_pcm_hw_params *params,
>  				   struct snd_pcm_hw_rule *rule)
>  {
> -	struct snd_pcm_hw_constraint_ratdens *r = rule->private;
> +	const struct snd_pcm_hw_constraint_ratdens *r = rule->private;
>  	unsigned int num = 0, den = 0;
>  	int err = snd_interval_ratden(hw_param_interval(params, rule->var),
>  				  r->nrats, r->rats, &num, &den);
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 53b5ada..5cc8c4f 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -1775,7 +1775,7 @@ const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
>  static int snd_pcm_hw_rule_rate(struct snd_pcm_hw_params *params,
>  				struct snd_pcm_hw_rule *rule)
>  {
> -	struct snd_pcm_hardware *hw = rule->private;
> +	const struct snd_pcm_hardware *hw = rule->private;
>  	return snd_interval_list(hw_param_interval(params, rule->var),
>  				 snd_pcm_known_rates.count,
>  				 snd_pcm_known_rates.list, hw->rates);
> @@ -1785,7 +1785,7 @@ static int snd_pcm_hw_rule_buffer_bytes_max(struct snd_pcm_hw_params *params,
>  					    struct snd_pcm_hw_rule *rule)
>  {
>  	struct snd_interval t;
> -	struct snd_pcm_substream *substream = rule->private;
> +	const struct snd_pcm_substream *substream = rule->private;
>  	t.min = 0;
>  	t.max = substream->buffer_bytes_max;
>  	t.openmin = 0;
> -- 
> 1.7.10
> 

Re: [PATCH] ALSA: pcm: Mostly constify constraints

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1020 bytes --]

On Thu, Jul 05, 2012 at 09:06:30AM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > There is no need for the constraints to be modified while being
> > applied

> Many drivers pass the local instance to rule->private, and it's not
> always guaranteed to be const.  For example, you can imagine some
> state modified kept in the struct while it's modified via rules.

> In short: you'd have seen many compile warnings for non-ASoC drivers
> if you run make once with this patch ;)

Well, that's trivial to fix.

> As a safe side fix, how about just adding const to
> snd_pcm_hw_constraint_list() & co, and expclitly cast to non-const
> later?

My inclination for something like this is to put the casts with the
users since that way we get the type safety through most of the code and
we can see that dropping the const is safe since we're just getting back
our own data.  If we can see the APIs immediately dropping the const I'd
expect we'd get people spending time on code review trying to figure out
if it's safe.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ALSA: pcm: Mostly constify constraints

[Takashi Iwai]

At Thu, 5 Jul 2012 10:28:42 +0100,
Mark Brown wrote:
> 
> On Thu, Jul 05, 2012 at 09:06:30AM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > There is no need for the constraints to be modified while being
> > > applied
> 
> > Many drivers pass the local instance to rule->private, and it's not
> > always guaranteed to be const.  For example, you can imagine some
> > state modified kept in the struct while it's modified via rules.
> 
> > In short: you'd have seen many compile warnings for non-ASoC drivers
> > if you run make once with this patch ;)
> 
> Well, that's trivial to fix.
> 
> > As a safe side fix, how about just adding const to
> > snd_pcm_hw_constraint_list() & co, and expclitly cast to non-const
> > later?
> 
> My inclination for something like this is to put the casts with the
> users since that way we get the type safety through most of the code and
> we can see that dropping the const is safe since we're just getting back
> our own data.  If we can see the APIs immediately dropping the const I'd
> expect we'd get people spending time on code review trying to figure out
> if it's safe.

The rule->private pointer isn't designed to be always const.  The data
purely depends on each rule function, thus whether it can be const or
not also depends on the rule, too.  For simple rules like list take
indeed the constant list.  But more complex rules may take a struct
containing variables changed on the fly (e.g. the max rate set by
user via control API).


Takashi

Re: [PATCH] ALSA: pcm: Mostly constify constraints

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 595 bytes --]

On Thu, Jul 05, 2012 at 11:55:36AM +0200, Takashi Iwai wrote:

> The rule->private pointer isn't designed to be always const.  The data
> purely depends on each rule function, thus whether it can be const or
> not also depends on the rule, too.  For simple rules like list take
> indeed the constant list.  But more complex rules may take a struct
> containing variables changed on the fly (e.g. the max rate set by
> user via control API).

Well, it's mostly const - it's const unless the rule itself starts
fiddling with stuff which I'd have expected to be the exception rather
than the rule.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ALSA: pcm: Mostly constify constraints

[Takashi Iwai]

At Thu, 5 Jul 2012 11:35:28 +0100,
Mark Brown wrote:
> 
> On Thu, Jul 05, 2012 at 11:55:36AM +0200, Takashi Iwai wrote:
> 
> > The rule->private pointer isn't designed to be always const.  The data
> > purely depends on each rule function, thus whether it can be const or
> > not also depends on the rule, too.  For simple rules like list take
> > indeed the constant list.  But more complex rules may take a struct
> > containing variables changed on the fly (e.g. the max rate set by
> > user via control API).
> 
> Well, it's mostly const - it's const unless the rule itself starts
> fiddling with stuff which I'd have expected to be the exception rather
> than the rule.

Well, the only rule is that rule (function) rules the meaning of the
private pointer :)


Takashi

Re: [PATCH] ALSA: pcm: Mostly constify constraints

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 389 bytes --]

On Thu, Jul 05, 2012 at 12:41:05PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > Well, it's mostly const - it's const unless the rule itself starts
> > fiddling with stuff which I'd have expected to be the exception rather
> > than the rule.

> Well, the only rule is that rule (function) rules the meaning of the
> private pointer :)

Ugh, that's pretty vile really.  But anyway...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]