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

[Dan Carpenter]

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