question about drivers/gpu/drm/gma500/oaktrail_lvds.c

[Patrik Jakobsson]

On Sun, Jul 8, 2012 at 10:16 PM, Alan Cox <alan at lxorguk.ukuu.org.uk> wrote:
> On Sun, 8 Jul 2012 10:39:43 +0200 (CEST)
> Julia Lawall <julia.lawall at lip6.fr> wrote:
>
>> In the function oaktrail_lvds_mode_set, I don't think that the following
>> code makes any sense:
>>
>>          /* Find the connector we're trying to set up */
>>          list_for_each_entry(connector, &mode_config->connector_list, head) {
>>                  if (!connector->encoder || connector->encoder->crtc != crtc)
>>                          continue;
>>          }
>>
>>          if (!connector) {
>>                  DRM_ERROR("Couldn't find connector when setting mode");
>>                  return;
>>          }
>>
>>          drm_connector_property_get_value(
>>                  connector,
>>                  dev->mode_config.scaling_mode_property,
>>                  &v);
>>
>> The initial loop is a no-op, because it always continues.  The test
>> !connector can never be true, because at the end of a list_for_each_entry
>> connector points to the list head, and calling
>> drm_connector_property_get_value on the list head probably does not make
>> sense.
>
> We test !connector->encoder rather than !connector ?

It seems we should break on :
if (connector->encoder && connector->encoder == encoder)

Then do a check after list iteration:
if (!connector || connector->encoder != encoder)
    DRM_ERROR("Couldn't find connector when setting mode");

But my attention span is less than a jiffie right now... will look at it
with fresh eyes tomorrow.

-Patrik