[PATCH i-g-t v3 2/2] tests/intel/i915_pm_rpm: check drmModeGetConnector() before use

[Kamil Konieczny]

Hi Luca,
On 2024-05-30 at 11:59:08 +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 init_mode_set_data(), so fix that.
> 
> From the bug report:
> 
> Starting subtest: module-reload
> Received signal SIGSEGV.
> Stack trace:

Now I think that line 'Stack trace' is not needed, only those
two with subtest name and signal are enough.

> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10911
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> Cc: Jani Saarinen <jani.saarinen at intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> Signed-off-by: Luca Coelho <luciano.coelho at intel.com>
> ---
>  tests/intel/i915_pm_rpm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/intel/i915_pm_rpm.c b/tests/intel/i915_pm_rpm.c
> index 0ea5fbd8afd9..e0f395fa8a84 100644
> --- a/tests/intel/i915_pm_rpm.c
> +++ b/tests/intel/i915_pm_rpm.c
> @@ -515,6 +515,12 @@ static void init_mode_set_data(struct mode_set_data *data)
>  			data->connectors[i] =
>  				drmModeGetConnector(drm_fd,
>  						    data->res->connectors[i]);
> +			if (!data->connectors[i]) {
> +				igt_warn("Could not read connector %u\n",
----------------^^^^^^^^
igt_debug

> +					 data->res->connectors[i]);
> +				continue;
> +			}
> +

We generally do not warn and fail a test in case some prerequistics
are not present, that is why it is better to igt_debug here and make
checks in few other places, like:
			if (data->connectors[i]) {
                    // here we can safely use/free connector or edid
            }
or
			if (!data->connectors[i])
                continue;

grep -n connectors tests/intel/i915_pm_rpm.c
139:    drmModeConnectorPtr connectors[MAX_CONNECTORS];
151:    drmModeConnectorPtr connectors[MAX_CONNECTORS];
319:    for (int i = 0; i < data->res->count_connectors; i++) {
320:            drmModeConnectorPtr c = data->connectors[i];
513:            igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
514:            for (int i = 0; i < data->res->count_connectors; i++) {
515:                    data->connectors[i] =
517:                                                data->res->connectors[i]);
518:                    data->edids[i] = get_connector_edid(data->connectors[i], i);
531:            for (int i = 0; i < data->res->count_connectors; i++) {
532:                    drmModeFreeConnector(data->connectors[i]);

so all you need is add checks in lines after 320 and 531.

Btw test is missing final fixture with i915 module load but that
should be fixed in separate patch.

Regards,
Kamil

>  			data->edids[i] = get_connector_edid(data->connectors[i], i);
>  		}
>  
> -- 
> 2.39.2
>