linux-kernel - Re: [PATCH 3/3] drm: bridge: anx78xx: Add anx78xx bridge driver support.

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: <20160324112818.GC5273@mwanda>
Date:	Thu, 24 Mar 2016 14:28:18 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	dri-devel@...ts.freedesktop.org, devel@...verdev.osuosl.org,
	treding@...dia.com, robh+dt@...nel.org, pawel.moll@....com,
	mark.rutland@....com, ijc+devicetree@...lion.org.uk,
	galak@...eaurora.org, airlied@...ux.ie, gregkh@...uxfoundation.org,
	sjoerd.simons@...labora.co.uk, javier@...hile0.org,
	span@...logixsemi.com, nathan.chung@...iatek.com,
	djkurtz@...omium.org, drinkcat@...omium.org,
	laurent.pinchart@...asonboard.com, jb.tsai@...iatek.com,
	cawa.cheng@...iatek.com, eddie.huang@...iatek.com,
	cjiao@...logixsemi.com, emil.l.velikov@...il.com
Subject: Re: [PATCH 3/3] drm: bridge: anx78xx: Add anx78xx bridge driver
 support.

On Thu, Mar 24, 2016 at 11:41:46AM +0100, Enric Balletbo i Serra wrote:
> +	/* Map slave addresses of ANX7814 */
> +	for (i = 0; i < I2C_NUM_ADDRESSES; i++) {
> +		anx78xx->i2c_dummy[i] = i2c_new_dummy(client->adapter,
> +						anx78xx_i2c_addresses[i] >> 1);
> +		if (!anx78xx->i2c_dummy[i]) {
> +			DRM_ERROR("Failed to reserve i2c bus %02x.\n",
> +				  anx78xx_i2c_addresses[i]);

Missing error code here.

> +			goto err_i2c;
> +		}

I'm, of course, not a fan of the naming.  The name should be based on
what the goto location does...  In this case it turns it off.  Which is
slightly weird because we have not turned it on yet...  I always say
that you should have multiple error labels and you only undo things
which have been done.

Having a common exit path for the other functions where it was "goto out"
makes sense.  But again in those cases I would prefer a meaningful label
name like "goto unlock;".  In the kernel "goto out;" is meaningless, it
could do anything or everything or nothing.  A lot of people like it
of course, but out: label code tends to be buggier than using a
meaningful name.

regards,
dan carpenter