linux-kernel - Re: [PATCH v3] ASOC: dapm: add code to configure dai link parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20140818185300.GK14537@sirena.org.uk>
Date:	Mon, 18 Aug 2014 13:53:00 -0500
From:	Mark Brown <broonie@...nel.org>
To:	Nikesh Oswal <nikesh@...nsource.wolfsonmicro.com>
Cc:	lgirdwood@...il.com, perex@...ex.cz, tiwai@...e.de,
	alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
	patches@...nsource.wolfsonmicro.com
Subject: Re: [PATCH v3] ASOC: dapm: add code to configure dai link parameters

On Wed, Aug 06, 2014 at 05:16:26PM +0100, Nikesh Oswal wrote:

> +/* create new dapm dai link control */
> +static int dapm_new_dai_link(struct snd_soc_dapm_widget *w)
> +{
> +	int i, ret;
> +	struct snd_kcontrol *kcontrol;
> +	struct snd_soc_dapm_context *dapm = w->dapm;
> +	struct snd_card *card = dapm->card->snd_card;
> +
> +
> +	/* add kcontrol */
> +	for (i = 0; i < w->num_kcontrols; i++) {
> +		kcontrol = snd_soc_cnew(&w->kcontrol_news[i], w,
> +		w->name, NULL);

We appear to be creating the control unconditionally so even a link
which has only one configuration will have a control created which
doesn't seem right.  Suppressing the control creation for links that
only have one configuration seems better.

Throughout this you've got coding style problems, you're not indenting
the continuations of multi-line statements.  Please try to follow the
kernel coding style, if your code is visibly different to the rest of
the codebase that's not good.

> +	for (count = 0 ; count < num_params; count++) {
> +		w_param_text[count] =  kzalloc(sizeof(char) * 100, GFP_KERNEL);

Random number!

> +		if (!w_param_text) {
> +			ret = -ENOMEM;
> +			goto  outfree_w_param;
> +		}
> +
> +		sprintf(w_param_text[count], "SampleRate=%d:Channels=%d:BitsPerSample=%d",
> +			config->rate_min, config->channels_min,
> +			snd_pcm_format_width(ffs(config->formats) - 1));
> +		config++;

The configurations have user speciable names in them which are being
ignored here - this means that users can't specify names that are
meaningful for their systems.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)