linux-kernel - Re: [PATCH v2 2/2] staging: iio: ad7780: moving ad7780 out of staging

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: <20190128133949.z6eryxwes7yty3l2@renatolg>
Date:   Mon, 28 Jan 2019 11:39:50 -0200
From:   Renato Lui Geh <renatogeh@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Renato Lui Geh <renatogeh@...il.com>, lars@...afoo.de,
        Michael.Hennerich@...log.com, jic23@...nel.org, knaack.h@....de,
        pmeerw@...erw.net, gregkh@...uxfoundation.org,
        stefan.popa@...log.com, alexandru.Ardelean@...log.com,
        giuliano.belinassi@....br, linux-iio@...r.kernel.org,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        kernel-usp@...glegroups.com
Subject: Re: [PATCH v2 2/2] staging: iio: ad7780: moving ad7780 out of staging

On 01/28, Dan Carpenter wrote:
>On Sun, Jan 27, 2019 at 06:33:52PM -0200, Renato Lui Geh wrote:
>> +	st->reg = devm_regulator_get(&spi->dev, "avdd");
>> +	if (IS_ERR(st->reg))
>> +		return PTR_ERR(st->reg);
>> +
>> +	ret = regulator_enable(st->reg);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
>> +		goto error_disable_reg;
>> +	}
>
>We don't need to disable the regulator if enabling it failed (probably,
>I haven't looked at the code).

I wasn't sure if ret would need to be cleaned up on failure, so I kept
it as it was before. I'll change it to a simple return if needed though.
>
>
>> +
>> +	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
>> +	if (ret)
>> +		goto error_disable_reg;
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret)
>> +		goto error_cleanup_buffer_and_trigger;
>> +
>> +	return 0;
>> +
>> +error_cleanup_buffer_and_trigger:
>> +	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>> +error_disable_reg:
>> +	regulator_disable(st->reg);
>> +
>
>Well chosen label names, btw.  Very easy to review.

That was already there before I came. But yes, very nice choice of
names. :)
>
>> +	return ret;
>> +}
>> +
>
>regards,
>dan carpenter