[PATCH xserver 1/4] xfree86: fix obscure bug in xf86platformProbeDev()'s retval computation

[Hans de Goede]

Hi,

On 04-09-16 03:11, Laszlo Ersek wrote:
> According to the leading comment on, and the implementation of,
> xf86CallDriverProbe(), the xf86platformProbeDev() function is supposed to
> return TRUE if the driver in question succeeds to probe at least one
> graphics card.
>
> The assignment to "foundScreen" in the "devList" loop of
> xf86platformProbeDev() doesn't calculate a logical disjunction however; a
> later probing failure currently suppresses earlier successes. Fix it.
>
> (The assignment to "foundScreen" in the "autoAddGPU" loop is correct
> already, no TRUE -> FALSE transition is possible there.)
>
> In addition, remove the useless "continue" statement at the end of the
> "devList" loop body.
>
> Cc: Adam Jackson <ajax at redhat.com>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Keith Packard <keithp at keithp.com>
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  hw/xfree86/common/xf86platformBus.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
> index 96895a6e17f4..9b796e01c537 100644
> --- a/hw/xfree86/common/xf86platformBus.c
> +++ b/hw/xfree86/common/xf86platformBus.c
> @@ -450,9 +450,8 @@ xf86platformProbeDev(DriverPtr drvp)
>          if (j == xf86_num_platform_devices)
>               continue;
>
> -        foundScreen = probeSingleDevice(&xf86_platform_devices[j], drvp, devList[i], 0);
> -        if (!foundScreen)
> -            continue;
> +        foundScreen |= probeSingleDevice(&xf86_platform_devices[j], drvp,
> +                                         devList[i], 0);
>      }
>
>      /* if autoaddgpu devices is enabled then go find any unclaimed platform

The autoAddGPU loop below this uses:

             if (probeSingleDevice(&xf86_platform_devices[j], drvp,
                                   devList ?  devList[0] : NULL,
                                   PLATFORM_PROBE_GPU_SCREEN))
                 foundScreen = TRUE;

I believe it would be better (more consistent to do the same here.
Also |= is a binary op, so it creates the suggestion that you're or-ing
in flags.

Regards,

Hans