linux-kernel - Re: [PATCH v2 5/9] ASoC: tegra: add Tegra210 based AHUB driver

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <bd802f93-dd03-f39d-86e1-39f8571bbbb0@gmail.com>
Date:   Thu, 30 Jan 2020 20:09:41 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sameer Pujar <spujar@...dia.com>, perex@...ex.cz, tiwai@...e.com,
        robh+dt@...nel.org
Cc:     broonie@...nel.org, lgirdwood@...il.com, thierry.reding@...il.com,
        jonathanh@...dia.com, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, sharadg@...dia.com,
        mkumard@...dia.com, viswanathl@...dia.com, rlokhande@...dia.com,
        dramesh@...dia.com, atalambedu@...dia.com
Subject: Re: [PATCH v2 5/9] ASoC: tegra: add Tegra210 based AHUB driver

30.01.2020 13:33, Sameer Pujar пишет:
...
> +	ret = devm_snd_soc_register_component(&pdev->dev,
> +					      ahub->soc_data->cmpnt_drv,
> +					      ahub->soc_data->dai_drv,
> +					      ahub->soc_data->num_dais);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register component, err: %d\n",
> +			ret);
> +		return ret;
> +	}
In the the patch #4 ("ASoC: tegra: add Tegra210 based I2S driver") I see
the following:

	ret = devm_snd_soc_register_component(dev, &tegra210_i2s_cmpnt,
					tegra210_i2s_dais,
					ARRAY_SIZE(tegra210_i2s_dais));
	if (ret != 0) {
		dev_err(dev, "can't register I2S component, err: %d\n", ret);
		return ret;
	}

Please be consistent in regards to errors checking. The correct variant
should be: if (ret != 0). Usually error codes are a negative value, but
it is much safer to check whether value isn't 0 in all cases where
positive value isn't expected to happen.

I'd also recommend to rename all "ret" variables to "err" everywhere in
the code where returned value is used only for errors checking. This
will make code more explicit, and hence, easier to read and follow.

So, it will be nicer to write it as:

	err = devm_snd_soc_register_component(&pdev->dev,
					ahub->soc_data->cmpnt_drv,
					ahub->soc_data->dai_drv,
					ahub->soc_data->num_dais);
	if (err) {
		dev_err(&pdev->dev, "failed to register component: %d\n", err);
		return err;
	}