Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Sousa <gustavo.sousa@intel.com>
To: igt-dev@lists.freedesktop.org
Subject: [PATCH i-g-t] runner: Remove useless null check for delim in job_list_from_test_list
Date: Tue, 30 Apr 2024 18:19:35 -0300	[thread overview]
Message-ID: <20240430211934.349775-2-gustavo.sousa@intel.com> (raw)

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>
---
 runner/job_list.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/runner/job_list.c b/runner/job_list.c
index 27cbb10bce56..aeabb59f49d3 100644
--- a/runner/job_list.c
+++ b/runner/job_list.c
@@ -276,16 +276,7 @@ static bool job_list_from_test_list(struct job_list *job_list,
 			 * specified, also start a new entry.
 			 */
 			if (entry.binary && !strcmp(entry.binary, binary) &&
-			    (delim == NULL || strchr(delim, '@') == NULL)) {
-				if (!delim) {
-					/* ... except we didn't get a subtest */
-					fprintf(stderr,
-						"Error: Unexpected test without subtests "
-						"after same test had subtests\n");
-					free(binary);
-					fclose(f);
-					return false;
-				}
+			    strchr(delim, '@') == NULL) {
 				entry.subtest_count++;
 				entry.subtests = realloc(entry.subtests,
 							 entry.subtest_count *
@@ -303,7 +294,7 @@ static bool job_list_from_test_list(struct job_list *job_list,
 
 			memset(&entry, 0, sizeof(entry));
 
-			if (delim != NULL && strchr(delim, '@') != NULL) {
+			if (strchr(delim, '@') != NULL) {
 				/* Dynamic subtest specified. Add to job list alone. */
 				char **subtests;
 
@@ -314,11 +305,9 @@ static bool job_list_from_test_list(struct job_list *job_list,
 				any = true;
 			} else {
 				entry.binary = strdup(binary);
-				if (delim) {
-					entry.subtests = malloc(sizeof(*entry.subtests));
-					entry.subtests[0] = strdup(delim);
-					entry.subtest_count = 1;
-				}
+				entry.subtests = malloc(sizeof(*entry.subtests));
+				entry.subtests[0] = strdup(delim);
+				entry.subtest_count = 1;
 			}
 
 			free(binary);
-- 
2.44.0


             reply	other threads:[~2024-04-30 21:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-30 21:19 Gustavo Sousa [this message]
2024-04-30 22:12 ` ✗ Fi.CI.BAT: failure for runner: Remove useless null check for delim in job_list_from_test_list Patchwork
2024-04-30 22:15 ` ✓ CI.xeBAT: success " Patchwork
2024-05-01  4:37 ` ✗ CI.xeFULL: failure " Patchwork
2024-05-07 17:13 ` [i-g-t] " Ngai-Mint Kwan
2024-05-17 21:32   ` Matt Roper
2024-05-10 17:37 ` [PATCH i-g-t] " Kamil Konieczny
2024-05-10 17:58   ` Gustavo Sousa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240430211934.349775-2-gustavo.sousa@intel.com \
    --to=gustavo.sousa@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox