-
The function job_list_from_test_list() uses a while loop to read entries from the testlist. The condition ((delim = strchr(binary, '@')) != NULL) checks if the current entry specifies a subtest. When no subtest is specified, the else clause will cause a jump to the next iteration. That means that we are certain any statement executed after that if block has (delim != NULL). As such, all null checks on that variable after that point are pointless. In fact, certain unnecessary null checks might even cause confusion to readers. One example is the block meant to display the "Unexpected test without subtests ..." message: it would never happen, since that condition is handled earlier in the iteration (i.e. the full set of subtests is expanded for that entry in the else clause previously mentioned). The following execution (done before this change) illustrates that: $ cat <<EOF > /tmp/foo.testlist igt@xe_pm@s2idle-basic igt@xe_pm igt@xe_pm@s2idle-exec-after EOF $ ./build/runner/igt_runner -d -m --test-list /tmp/foo.testlist build/tests /tmp/results [36408.109043] Dry run, not executing. Invoke igt_resume if you want to execute. Done. $ cat /tmp/results/joblist.txt | sed 's/^\(.\{50\}\).\+/\1.../' xe_pm s2idle-basic xe_pm s2idle-basic,s2idle-basic-exec,s2idle-exec-a... xe_pm s2idle-exec-after Let's remove those unnecessary checks. Signed-off-by:Gustavo Sousa <gustavo.sousa@intel.com> Reviewed-by:
Ngai-Mint Kwan <ngai-mint.kwan@linux.intel.com> Link: https://lore.kernel.org/r/20240430211934.349775-2-gustavo.sousa@intel.com Signed-off-by:
Matt Roper <matthew.d.roper@intel.com>
The function job_list_from_test_list() uses a while loop to read entries from the testlist. The condition ((delim = strchr(binary, '@')) != NULL) checks if the current entry specifies a subtest. When no subtest is specified, the else clause will cause a jump to the next iteration. That means that we are certain any statement executed after that if block has (delim != NULL). As such, all null checks on that variable after that point are pointless. In fact, certain unnecessary null checks might even cause confusion to readers. One example is the block meant to display the "Unexpected test without subtests ..." message: it would never happen, since that condition is handled earlier in the iteration (i.e. the full set of subtests is expanded for that entry in the else clause previously mentioned). The following execution (done before this change) illustrates that: $ cat <<EOF > /tmp/foo.testlist igt@xe_pm@s2idle-basic igt@xe_pm igt@xe_pm@s2idle-exec-after EOF $ ./build/runner/igt_runner -d -m --test-list /tmp/foo.testlist build/tests /tmp/results [36408.109043] Dry run, not executing. Invoke igt_resume if you want to execute. Done. $ cat /tmp/results/joblist.txt | sed 's/^\(.\{50\}\).\+/\1.../' xe_pm s2idle-basic xe_pm s2idle-basic,s2idle-basic-exec,s2idle-exec-a... xe_pm s2idle-exec-after Let's remove those unnecessary checks. Signed-off-by:
