[LTP] [PATCH v3 0/4] lib: LTP_DEBUG cleanup

[LTP] [PATCH v3 0/4] lib: LTP_DEBUG cleanup

[Petr Vorel]

Changes v2->v3:
* C cleanup (tdebug_env[0] != '\0' => *tdebug_env, thanks Li)
* Simplify -D code (Li)
* New commit "doc: newlib_tests: Update debugging parameters"

Link to v2:
https://patchwork.ozlabs.org/project/ltp/list/?series=497627&state=*
https://lore.kernel.org/ltp/20260327154603.GC1092845@pevik/T/#t


Petr Vorel (4):
  lib: Ignore empty LTP_ENABLE_DEBUG
  lib: Rename variable LTP_ENABLE_DEBUG => LTP_DEBUG
  lib: Prefer LTP_DEBUG over -D
  doc: newlib_tests: Update debugging parameters

 doc/developers/debugging.rst     |  6 ++++--
 doc/old/C-Test-API.asciidoc      |  4 ++--
 doc/users/setup_tests.rst        |  4 ++--
 lib/newlib_tests/tst_res_flags.c |  2 +-
 lib/tst_test.c                   | 33 +++++++++++++++-----------------
 5 files changed, 24 insertions(+), 25 deletions(-)

-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 1/4] lib: Ignore empty LTP_ENABLE_DEBUG

[Petr Vorel]

Empty LTP_ENABLE_DEBUG environment variable should be just ignored.

This fixes warning:
LTP_ENABLE_DEBUG= ./test05
tst_test.c:1454: TWARN: Invalid LTP_ENABLE_DEBUG value:

Reviewed-by: Li Wang <liwang@redhat.com>
Fixes: e434de62b4 ("lib: Extend LTP_ENABLE_DEBUG to support verbosity levels")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v2->v3:
* C cleanup (tdebug_env[0] != '\0' => *tdebug_env, thanks Li)

 lib/tst_test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2d62ab164d..6cd5546c0f 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1445,13 +1445,13 @@ static void do_setup(int argc, char *argv[])
 
 	parse_opts(argc, argv);
 
-	if (tdebug_env && !context->tdebug) {
+	if (tdebug_env && *tdebug_env && !context->tdebug) {
 		if (!strcmp(tdebug_env, "2"))
 			context->tdebug = 2;
 		else if (!strcmp(tdebug_env, "1") || !strcmp(tdebug_env, "y"))
 			context->tdebug = 1;
 		else
-			tst_res(TWARN, "Invalid LTP_ENABLE_DEBUG value: %s", tdebug_env);
+			tst_res(TWARN, "Invalid LTP_ENABLE_DEBUG value: '%s'", tdebug_env);
 
 		if (context->tdebug)
 			tst_res(TINFO, "Enabling debug info (level %d)", context->tdebug);
-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 2/4] lib: Rename variable LTP_ENABLE_DEBUG => LTP_DEBUG

[Petr Vorel]

1) LTP_DEBUG is shorter to write.
2) Enable is usually meant as boolean (on/off), that hides possible 2
value even more.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/developers/debugging.rst     | 2 +-
 doc/old/C-Test-API.asciidoc      | 2 +-
 doc/users/setup_tests.rst        | 2 +-
 lib/newlib_tests/tst_res_flags.c | 2 +-
 lib/tst_test.c                   | 6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/doc/developers/debugging.rst b/doc/developers/debugging.rst
index 52f881c5c8..36dda0d90f 100644
--- a/doc/developers/debugging.rst
+++ b/doc/developers/debugging.rst
@@ -9,7 +9,7 @@ Debug messages
 --------------
 
 The LTP framework supports ``TDEBUG`` flag test debug messages. These
-messages can be enabled using the ``-D[1,2]`` parameter or setting ``LTP_ENABLE_DEBUG=1,2``
+messages can be enabled using the ``-D[1,2]`` parameter or setting ``LTP_DEBUG=1,2``
 environment variable (see :doc:`../users/setup_tests`).
 
 The ``-D`` parameter also supports the following verbosity levels:
diff --git a/doc/old/C-Test-API.asciidoc b/doc/old/C-Test-API.asciidoc
index 72fd2731d3..4207b28e92 100644
--- a/doc/old/C-Test-API.asciidoc
+++ b/doc/old/C-Test-API.asciidoc
@@ -232,7 +232,7 @@ Printf-like function to report test result, it's mostly used with ttype:
 | 'TPASS'  | Test has passed.
 | 'TFAIL'  | Test has failed.
 | 'TINFO'  | General message.
-| 'TDEBUG' | Debug message (new C API only, printed with '-D' or via 'LTP_ENABLE_DEBUG=1' or 'y'
+| 'TDEBUG' | Debug message (new C API only, printed with '-D' or via 'LTP_DEBUG=1' or 'y'
              environment variable), only for messages which would be too verbose for normal run.
 | 'TWARN'  | Something went wrong but we decided to continue. Mostly used in cleanup functions.
 |==============================
diff --git a/doc/users/setup_tests.rst b/doc/users/setup_tests.rst
index 95d4d9e380..a42c397feb 100644
--- a/doc/users/setup_tests.rst
+++ b/doc/users/setup_tests.rst
@@ -90,7 +90,7 @@ users.
      - Disable running test cleanup (defined in ``TST_CLEANUP``).
        Shell API only.
 
-   * - LTP_ENABLE_DEBUG
+   * - LTP_DEBUG
      - Enable debug info (value ``1`` (``y``) or ``2`` for more verbose level).
        Equivalent of ``-D`` parameter.
 
diff --git a/lib/newlib_tests/tst_res_flags.c b/lib/newlib_tests/tst_res_flags.c
index 21637a472b..9993d0f87d 100644
--- a/lib/newlib_tests/tst_res_flags.c
+++ b/lib/newlib_tests/tst_res_flags.c
@@ -21,7 +21,7 @@ static struct tcase {
 	{FLAG(TCONF)},
 	{FLAG(TWARN)},
 	{FLAG(TINFO)},
-	{FLAG(TDEBUG), " (printed only with -D[1,2] or LTP_ENABLE_DEBUG=1,2)"},
+	{FLAG(TDEBUG), " (printed only with -D[1,2] or LTP_DEBUG=1,2)"},
 };
 
 static void do_cleanup(void)
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 6cd5546c0f..ea1aa5d306 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -675,7 +675,7 @@ static void print_help(void)
 	fprintf(stderr, "LTP_COLORIZE_OUTPUT      Force colorized output behaviour (y/1 always, n/0: never)\n");
 	fprintf(stderr, "LTP_DEV                  Path to the block device to be used (for .needs_device)\n");
 	fprintf(stderr, "LTP_DEV_FS_TYPE          Filesystem used for testing (default: %s)\n", DEFAULT_FS_TYPE);
-	fprintf(stderr, "LTP_ENABLE_DEBUG         Print debug messages (set 1(y) or 2)\n");
+	fprintf(stderr, "LTP_DEBUG                Print debug messages (set 1(y) or 2)\n");
 	fprintf(stderr, "LTP_REPRODUCIBLE_OUTPUT  Values 1 or y discard the actual content of the messages printed by the test\n");
 	fprintf(stderr, "LTP_QUIET                Values 1 or y will suppress printing TCONF, TWARN, TINFO, and TDEBUG messages\n");
 	fprintf(stderr, "LTP_SINGLE_FS_TYPE       Specifies filesystem instead all supported (for .all_filesystems)\n");
@@ -1400,7 +1400,7 @@ bool tst_cmd_present(const char *cmd)
 
 static void do_setup(int argc, char *argv[])
 {
-	char *tdebug_env = getenv("LTP_ENABLE_DEBUG");
+	char *tdebug_env = getenv("LTP_DEBUG");
 	char *reproducible_env = getenv("LTP_REPRODUCIBLE_OUTPUT");
 	char *quiet_env = getenv("LTP_QUIET");
 
@@ -1451,7 +1451,7 @@ static void do_setup(int argc, char *argv[])
 		else if (!strcmp(tdebug_env, "1") || !strcmp(tdebug_env, "y"))
 			context->tdebug = 1;
 		else
-			tst_res(TWARN, "Invalid LTP_ENABLE_DEBUG value: '%s'", tdebug_env);
+			tst_res(TWARN, "Invalid LTP_DEBUG value: '%s'", tdebug_env);
 
 		if (context->tdebug)
 			tst_res(TINFO, "Enabling debug info (level %d)", context->tdebug);
-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 3/4] lib: Prefer LTP_DEBUG over -D

[Petr Vorel]

Environment variable has usually higher preference than getopt.
This includes disabling debugging even on -D:
LTP_DEBUG= ./test05 -D2 means debugging disabled.

Also reduce duplicity of "Enabling debug info (level %d)" message and
document that on -h and developers/debugging.rst.

Fixes: e434de62b4 ("lib: Extend LTP_ENABLE_DEBUG to support verbosity levels")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v2->v3:
* Simplify -D code (Li)

 doc/developers/debugging.rst |  2 ++
 lib/tst_test.c               | 25 +++++++++++--------------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/doc/developers/debugging.rst b/doc/developers/debugging.rst
index 36dda0d90f..4e162068b4 100644
--- a/doc/developers/debugging.rst
+++ b/doc/developers/debugging.rst
@@ -19,6 +19,8 @@ The ``-D`` parameter also supports the following verbosity levels:
 
 Suppress all debug logs if no '-D' flag passed (default behavior).
 
+``LTP_DEBUG`` has higher preference than ``-D``.
+
 Tracing and debugging syscalls
 ------------------------------
 
diff --git a/lib/tst_test.c b/lib/tst_test.c
index ea1aa5d306..61e357af3f 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -654,11 +654,11 @@ static struct option {
 	char *optstr;
 	char *help;
 } options[] = {
-	{"h",  "-h       Prints this help"},
-	{"i:", "-i n     Execute test n times"},
-	{"I:", "-I x     Execute test for n seconds"},
-	{"D::", "-D[1,2]  Prints debug information"},
-	{"V",  "-V       Prints LTP version"},
+	{"h",  "-h        Prints this help"},
+	{"i:", "-i n      Execute test n times"},
+	{"I:", "-I x      Execute test for n seconds"},
+	{"D::", "-D[1,2]  Prints debug information (can be overwriten by LTP_DEBUG"},
+	{"V",  "-V        Prints LTP version"},
 };
 
 static void print_help(void)
@@ -825,13 +825,9 @@ static void parse_opts(int argc, char *argv[])
 			tst_brk(TBROK, "Invalid option");
 		break;
 		case 'D':
-			if (optarg)
-				context->tdebug = SAFE_STRTOL(optarg, 1, 2);
-			else
-				context->tdebug = 1;
-
-			if (context->tdebug)
-				tst_res(TINFO, "Enabling debug info (level %d)", context->tdebug);
+			if (getenv("LTP_DEBUG"))
+				break;
+			context->tdebug = optarg ? SAFE_STRTOL(optarg, 1, 2) : 1;
 		break;
 		case 'h':
 			print_help();
@@ -1453,10 +1449,11 @@ static void do_setup(int argc, char *argv[])
 		else
 			tst_res(TWARN, "Invalid LTP_DEBUG value: '%s'", tdebug_env);
 
-		if (context->tdebug)
-			tst_res(TINFO, "Enabling debug info (level %d)", context->tdebug);
 	}
 
+	if (context->tdebug)
+		tst_res(TINFO, "Enabling debug info (level %d)", context->tdebug);
+
 	if (tst_test->needs_kconfigs && tst_kconfig_check(tst_test->needs_kconfigs))
 		tst_brk(TCONF, "Aborting due to unsuitable kernel config, see above!");
 
-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 4/4] doc: newlib_tests: Update debugging parameters

[Petr Vorel]

* Improve wording in doc/developers/debugging.rst. Not mention 'y'
  (enough for user is to know 1 and 2 + 'y' is mentioned at -h).
* Mention all -D and LTP_DEBUG parameters in library test.
* Keep info about the levels in developers/debugging.rst, link page in
  user/setup_tests.
* Remove values from old doc (we have enough places to sync on changes,
  let's drop the old doc).

Follow-up: b415265cb3 ("doc/setup_tests: Document LTP_ENABLE_DEBUG=2")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v3.

 doc/developers/debugging.rst     | 4 ++--
 doc/old/C-Test-API.asciidoc      | 4 ++--
 doc/users/setup_tests.rst        | 2 +-
 lib/newlib_tests/tst_res_flags.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/doc/developers/debugging.rst b/doc/developers/debugging.rst
index 4e162068b4..43d142dfeb 100644
--- a/doc/developers/debugging.rst
+++ b/doc/developers/debugging.rst
@@ -9,10 +9,10 @@ Debug messages
 --------------
 
 The LTP framework supports ``TDEBUG`` flag test debug messages. These
-messages can be enabled using the ``-D[1,2]`` parameter or setting ``LTP_DEBUG=1,2``
+messages can be enabled using the ``-D`` parameter or setting ``LTP_DEBUG``
 environment variable (see :doc:`../users/setup_tests`).
 
-The ``-D`` parameter also supports the following verbosity levels:
+Both ``-D`` parameter and ``LTP_DEBUG``  support the following verbosity levels:
 
   ``-D1`` (or ``-D``): Enable debug logs for the test process only.
   ``-D2``: Enable verbose debug logs for both the test and library processes.
diff --git a/doc/old/C-Test-API.asciidoc b/doc/old/C-Test-API.asciidoc
index 4207b28e92..503dc91ce5 100644
--- a/doc/old/C-Test-API.asciidoc
+++ b/doc/old/C-Test-API.asciidoc
@@ -232,8 +232,8 @@ Printf-like function to report test result, it's mostly used with ttype:
 | 'TPASS'  | Test has passed.
 | 'TFAIL'  | Test has failed.
 | 'TINFO'  | General message.
-| 'TDEBUG' | Debug message (new C API only, printed with '-D' or via 'LTP_DEBUG=1' or 'y'
-             environment variable), only for messages which would be too verbose for normal run.
+| 'TDEBUG' | Debug message (new C API only, enabled via '-D' or 'LTP_DEBUG' environment variable)
+             only for messages which would be too verbose for normal run.
 | 'TWARN'  | Something went wrong but we decided to continue. Mostly used in cleanup functions.
 |==============================
 
diff --git a/doc/users/setup_tests.rst b/doc/users/setup_tests.rst
index a42c397feb..6f1d996e18 100644
--- a/doc/users/setup_tests.rst
+++ b/doc/users/setup_tests.rst
@@ -92,7 +92,7 @@ users.
 
    * - LTP_DEBUG
      - Enable debug info (value ``1`` (``y``) or ``2`` for more verbose level).
-       Equivalent of ``-D`` parameter.
+       Equivalent of the ``-D`` parameter (see :doc:`../developers/debugging`).
 
 Test specific environment variables
 -----------------------------------
diff --git a/lib/newlib_tests/tst_res_flags.c b/lib/newlib_tests/tst_res_flags.c
index 9993d0f87d..cda0970702 100644
--- a/lib/newlib_tests/tst_res_flags.c
+++ b/lib/newlib_tests/tst_res_flags.c
@@ -21,7 +21,7 @@ static struct tcase {
 	{FLAG(TCONF)},
 	{FLAG(TWARN)},
 	{FLAG(TINFO)},
-	{FLAG(TDEBUG), " (printed only with -D[1,2] or LTP_DEBUG=1,2)"},
+	{FLAG(TDEBUG), " (printed only with -D[1,2] or LTP_DEBUG=1(y),2)"},
 };
 
 static void do_cleanup(void)
-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/4] lib: Prefer LTP_DEBUG over -D

[Andrea Cervesato via ltp]

Hi Petr,

> +	{"D::", "-D[1,2]  Prints debug information (can be overwriten by LTP_DEBUG"},

Issue is still here.
Typo: "overwriten" -> "overwritten". Also missing closing ')'.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 4/4] doc: newlib_tests: Update debugging parameters

[Andrea Cervesato via ltp]

Hi Petr,

> +Both ``-D`` parameter and ``LTP_DEBUG``  support the following verbosity levels:

Double space before "support".

The rest looks good:

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/4] lib: Ignore empty LTP_ENABLE_DEBUG

[Andrea Cervesato via ltp]

LGTM

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/4] lib: Rename variable LTP_ENABLE_DEBUG => LTP_DEBUG

[Andrea Cervesato via ltp]

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp