[PATCH i-g-t 1/2] lib/igt_chamelium: check drmModeGetConnector() return value in wait_for_connected_state()

[Kamil Konieczny]

Hi Luca,
On 2024-05-23 at 11:44:25 +0300, Luca Coelho wrote:
> The drmModeGetConnector() function can return NULL in some cases, so
> we need to check the return value before accessing it.  This is not
> being checked in wait_for_connected_state(), so fix that.
> 
> Signed-off-by: Luca Coelho <luciano.coelho at intel.com>
> ---
>  lib/igt_chamelium.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 016d5356630c..b1478654d02a 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -524,6 +524,9 @@ static bool wait_for_connected_state(int drm_fd,
>  			drmModeConnector *connector =
>  				drmModeGetConnector(drm_fd, connectors[i]);
>  
> +			if (!connector)
> +				break;

Why break? btw please document what is that function
tring to do, it looks strange.

Is it:
wait for one connector to enter connected state
or
wait for all connectors to be connected?

See below.

> +
>  			connected = connector->connection == DRM_MODE_CONNECTED;
>  
>  			drmModeFreeConnector(connector);
> -- 
> 2.39.2
> 

Some comments on this function code:

igt_until_timeout(CHAMELIUM_HOTPLUG_DETECTION_DELAY) {
    bool connected;

    for (int i = 0; i < connector_count; i++) {
        drmModeConnector *connector = drmModeGetConnector(drm_fd, connectors[i]);

		if (!connector)
				continue;

        connected = connector->connection == DRM_MODE_CONNECTED;
        drmModeFreeConnector(connector);

        if (!connected)
------------^
            break;
------------^
Why break here? imho this should be:

        if (connected)
            return true;

        }

    if (connected)
------------^
        return true;

This if(..)... could be then deleted.

    usleep(50000);
}

Regards,
Kamil