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

[Kamil Konieczny]

Hi Luca,
On 2024-05-28 at 10:58:54 +0300, Luca Coelho wrote:

change now looks ok but why did you ignored my suggestion
about short subject? Please see CONTRIBUTING.md about test
description:
The description should contain the spirit of the test (what is
the general idea behind the test) and *not* the letter (C to English
translation of the test).


The same principle apply to your subject description of change,
it should be short, not 'translation from C code of a change'.
It should also be rather short, up to 75 chars (or maybe even 65)
to keep it all in mailing list discussion.
You coould also look into:
https://patchwork.freedesktop.org/series/134188/

So imho here, instead of:
lib/igt_chamelium: check drmModeGetConnector() return in wait_for_connected_state()

imho better:
lib/igt_chamelium: Fix NULL dereferencing bug in wait_for_connected_state()

or even better:
lib/igt_chamelium: Fix bug in wait_for_connected_state()

or maybe:
lib/igt_chamelium: check drmModeGetConnector() return value before use

You could propose other subject if you find a better description.

Regards,
Kamil

> 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.
> 
> Cc: Mark Yacoub <markyacoub at chromium.org>
> Cc: Manasi Navare <navaremanasi at chromium.org>
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho at intel.com>
> ---
>  lib/igt_chamelium.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 016d5356630c..620fbbf7d94f 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -524,6 +524,11 @@ static bool wait_for_connected_state(int drm_fd,
>  			drmModeConnector *connector =
>  				drmModeGetConnector(drm_fd, connectors[i]);
>  
> +			if (!connector) {
> +				connected = false;
> +				break;
> +			}
> +
>  			connected = connector->connection == DRM_MODE_CONNECTED;
>  
>  			drmModeFreeConnector(connector);
> -- 
> 2.39.2
>