[Intel-gfx] [Resend][PATCH 3/4] drm/i915: Construct all possible sdvo outputs for one sdvo port

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ma, Ling wrote:
> 
>> -----Original Message-----
>> From: Ian Romanick [mailto:idr at freedesktop.org]
>> Sent: 2009年6月30日 0:35
>> To: Ma, Ling
>> Cc: intel-gfx at lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [Resend][PATCH 3/4] drm/i915: Construct all possible
>> sdvo outputs for one sdvo port
>>
> ling.ma at intel.com wrote:
>>>> +bool intel_sdvo_init(struct drm_device *dev, int output_device)
>>>> +{
>>>> +	int sdvo_output_id;
>>>> +	int sdvo_output_flags = MAX_SDVO_OUTPUTS_BITS;
>>>> +	bool ret = false;
>>>> +
>>>> +	for (sdvo_output_id = 1; sdvo_output_flags > 0; sdvo_output_id++) {
>>>> +
>>>> +		sdvo_output_flags =
>>>> +			intel_sdvo_outputs_init(dev, output_device,
>>>> +						sdvo_output_id,
>>>> +						sdvo_output_flags);
>>>> +		if (sdvo_output_flags >= 0)
>>>> +			ret = true;
>>>> +	}
> What if intel_sdvo_outputs_init never returns success?  It would be bad
> to have an infinite loop in the kernel. :)
>> Once return -1 from intel_sdvo_outputs_init, sdvo_output_flags will be less or equal to 0, program will exit. 

I missed that.  It's a bad idea to have the initializer and the
increment in the for-statement operate on something different than what
the test operates on.  It's easy for people to misread the code.

This really should be:

	sdvo_output_id = 1;

	/* Iterate over output devices as long as <explain what a return
	 * value greater than zero from intel_sdvo_outputs_init means>.
	 */
	do {
		sdvo_output_flags =
			intel_sdvo_outputs_init(dev, output_device,
						sdvo_output_id++,
						sdvo_output_flags);

		/* Some device was initialized.  We will eventually
		 * return success.
		 */
		if (sdvo_output_flags >= 0)
			ret = true;

	} while (sdvo_output_flags > 0);

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpKUc4ACgkQX1gOwKyEAw+WXgCfdyFwJWaLlzd6Ra1zMLOCbfaC
2iMAnj94Ri0IdBX3JM9SNyCImVgBqTpP
=GV+e
-----END PGP SIGNATURE-----