Re: [PATCH i-g-t] tests/kms_atomic_transition: Fix dynamic subtest

[Ngai-Mint Kwan <ngai-mint.kwan@linux.intel.com>]

Hi Gustavo,

On 2025-10-27 07:15, Gustavo Sousa wrote:
> Quoting Ngai-Mint Kwan (2025-10-24 20:37:28-03:00)
>> Fix Segmentation Fault when calling igt_pipe_crc_free() due to double-freeing a
>> dangling pointer.
>>
>> This occurs when a system connected to multiple displays executes a test with
>> "1x-outputs" dynamic subtest.
>>
>> Example:
>> kms_atomic_transition --run-subtest modeset-transition --dynamic-subtest 1x-outputs
>>
>> Signed-off-by: Ngai-Mint Kwan <ngai-mint.kwan@linux.intel.com>
>> ---
>> tests/kms_atomic_transition.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>> index a13830965..a7d8d88b4 100644
>> --- a/tests/kms_atomic_transition.c
>> +++ b/tests/kms_atomic_transition.c
>> @@ -1065,8 +1065,7 @@ static void run_modeset_transition(data_t *data, int requested_outputs, bool non
>>                  return;
>>          }
>>
>> -        igt_dynamic_f("%ix-outputs", requested_outputs)
>> -                run_modeset_tests(data, requested_outputs, nonblocking, fencing);
>> +        run_modeset_tests(data, requested_outputs, nonblocking, fencing);
>>
>>          /* Cleanup */
>>          unset_output_pipe(&data->display);
>> @@ -1259,9 +1258,10 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>>                  igt_describe_f("%s", modeset_tests[i].desc);
>>                  igt_subtest_with_dynamic_f("%s", modeset_tests[i].name) {
>>                          for (j = 1; j <= count; j++) {
>> -                                run_modeset_transition(&data, j,
>> -                                                       modeset_tests[i].nonblocking,
>> -                                                       modeset_tests[i].fencing);
>> +                                igt_dynamic_f("%ix-outputs", j)
>> +                                        run_modeset_transition(&data, j,
>> +                                                               modeset_tests[i].nonblocking,
>> +                                                               modeset_tests[i].fencing);
> I believe one possible problem here is that we might end up creating a
> dynamic subtest that ends not not getting exercised, when (num_outputs <
> requested_outputs) evaluates to true inside run_modeset_transition().
I looked at run_modeset_transition() and it looks more like an exit 
criteria
rather than a valid subtest when it is checking for (num_outputs < 
requested_outputs)?

Kwan
>
> Maybe we should rather move the cleanup code to be contained inside
> run_modeset_tests()?
>
> --
> Gustavo Sousa
>
>>                          }
>>                  }
>>          }
>> -- 
>> 2.43.0
>>