Re: [PATCH i-g-t v3 2/3] tests/intel/kms_dp_linktrain_fallback: fix MST traversal state leak between subtests

"Joshi, Kunal1" <[email protected]>
Newsgroups org.freedesktop.lists.igt-dev
Message-ID <[email protected]>
Hello Sowmiya,

On 05-08-2026 14:14, Sowmiya S wrote:
> traversed_mst_output_count is file-static and was never reset between
> subtests. In a full-binary run the count from dp-fallback causes
> setup_mst_outputs() to skip all MST outputs in subsequent subtests.
>
> Reset it at the top of run_lt_fallback_test(). The DSC path never
> reaches setup_mst_outputs() so run_dsc_sst_fallaback_test() does not
> need the reset.
In the tree this patch applies to there are exactly two subtests, 
dp-fallback
and dsc-fallback. setup_mst_outputs() is only reachable from setup_outputs()
(tests/intel/kms_dp_linktrain_fallback.c:184), setup_outputs() is only 
called
from test_fallback() (:365), and test_fallback() is only called from
run_lt_fallback_test(). So run_lt_fallback_test() is the only path that ever
touches traversed_mst_output_count, and igt_main() calls it exactly 
once. The
counter starts at 0 (file-static), and as you say yourself in the next
paragraph the DSC path never reaches setup_mst_outputs().

So "the count from dp-fallback causes setup_mst_outputs() to skip all MST
outputs in subsequent subtests" describes something that cannot happen, and
this patch is a no-op standalone. It only becomes live in 3/3, where
run_lt_fallback_test() acquires a second caller.

That is fine for a prep patch - please just say so:

         run_lt_fallback_test() tracks which MST outputs it has already
         traversed in a file-static counter that is never cleared. Today it
         has a single caller so the counter is implicitly zero, but a second
         caller would silently inherit the first run's state and skip every
         MST sibling as "already visited".

         Reset the counter on entry so the function is safe to call more 
than
         once per binary. The DSC path never reaches setup_mst_outputs(), so
         run_dsc_sst_fallaback_test() does not need the same treatment.


Thanks and Regards
Kunal Joshi

>
> Signed-off-by: Sowmiya S <[email protected]>
> ---
>   tests/intel/kms_dp_linktrain_fallback.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
> index 115183d2d..44b875ab9 100644
> --- a/tests/intel/kms_dp_linktrain_fallback.c
> +++ b/tests/intel/kms_dp_linktrain_fallback.c
> @@ -431,6 +431,13 @@ static bool run_lt_fallback_test(data_t *data)
>   	bool ran = false;
>   	igt_output_t *output;
>   
> +	/*
> +	 * Reset per invocation so MST traversal state from a previous subtest
> +	 * (e.g. dp-fallback followed by dsc-fallback) doesn't leak and cause
> +	 * MST siblings to be silently skipped as "already visited".
> +	 */
> +	traversed_mst_output_count = 0;
> +
>   	for_each_connected_output(&data->display, output) {
>   		data->output = output;
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.