[LTP] [PATCH v2 0/3] lib: LTP_DEBUG cleanup

[LTP] [PATCH v2 0/3] lib: LTP_DEBUG cleanup

[Petr Vorel]

Changes v1->v2:
* 2 more commits (first commit is the same, I'm sorry for the noise)

Petr Vorel (3):
  lib: Ignore empty LTP_ENABLE_DEBUG
  lib: Rename variable LTP_ENABLE_DEBUG => LTP_DEBUG
  lib: Prefer LTP_DEBUG over -D

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

-- 
2.53.0


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

[LTP] [PATCH v2 1/3] 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:

Fixes: e434de62b4 ("lib: Extend LTP_ENABLE_DEBUG to support verbosity levels")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 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..4a22f2e477 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[0] != '\0' && !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 v2 2/3] 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.

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 4a22f2e477..6b41c6e28f 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 v2 3/3] lib: Prefer LTP_DEBUG over -D

[Petr Vorel]

Environment variable has usually higher preference than getopt.
Also document it.

Fixes: e434de62b4 ("lib: Extend LTP_ENABLE_DEBUG to support verbosity levels")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/users/setup_tests.rst |  2 +-
 lib/tst_test.c            | 26 ++++++++++++++------------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/doc/users/setup_tests.rst b/doc/users/setup_tests.rst
index a42c397feb..7e7fb9f24c 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 ``-D`` parameter (but LTP_DEBUG has higher preference).
 
 Test specific environment variables
 -----------------------------------
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 6b41c6e28f..21a299f98f 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,15 @@ 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")) {
+				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);
+			}
 		break;
 		case 'h':
 			print_help();
-- 
2.53.0


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

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

[Li Wang via ltp]

On Thu, Mar 26, 2026 at 06:04:35PM +0100, Petr Vorel wrote:
> Environment variable has usually higher preference than getopt.
> Also document it.
> 
> Fixes: e434de62b4 ("lib: Extend LTP_ENABLE_DEBUG to support verbosity levels")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Reviewed-by: Li Wang <liwang@redhat.com>

> @@ -825,13 +825,15 @@ 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")) {

My preference is to shorten the code indentation:

  case 'D':
      if (getenv("LTP_DEBUG"))
          break;
  
      context->tdebug = optarg ? SAFE_STRTOL(optarg, 1, 2) : 1;

      if (context->tdebug)
          tst_res(TINFO, "Enabling debug info (level %d)", context->tdebug);
  break;

> +				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);
> +			}
>  		break;
>  		case 'h':
>  			print_help();
> -- 
> 2.53.0
> 

-- 
Regards,
Li Wang


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

Re: [LTP] [PATCH v2 2/3] lib: Rename variable LTP_ENABLE_DEBUG => LTP_DEBUG

[Li Wang via ltp]

On Thu, Mar 26, 2026 at 06:04:34PM +0100, Petr Vorel wrote:
> 1) LTP_DEBUG is shorter to write.
> 2) Enable is usually meant as boolean (on/off), that hides possible 2
> value even more.

Agreed, LTP_DEBUG is better.

Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang


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

Re: [LTP] [PATCH v2 1/3] lib: Ignore empty LTP_ENABLE_DEBUG

[Li Wang via ltp]

On Thu, Mar 26, 2026 at 06:04:33PM +0100, Petr Vorel wrote:
> 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:
> 
> Fixes: e434de62b4 ("lib: Extend LTP_ENABLE_DEBUG to support verbosity levels")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Reviewed-by: Li Wang <liwang@redhat.com>

> ---
>  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..4a22f2e477 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[0] != '\0' && !context->tdebug) {

Or, more shorten:

    if (tdebug_env && *tdebug_env && !context->tdebug) {

-- 
Regards,
Li Wang


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

Re: [LTP] [PATCH v2 1/3] lib: Ignore empty LTP_ENABLE_DEBUG

[Petr Vorel]

Hi Li,

> > -	if (tdebug_env && !context->tdebug) {
> > +	if (tdebug_env && tdebug_env[0] != '\0' && !context->tdebug) {

> Or, more shorten:

>     if (tdebug_env && *tdebug_env && !context->tdebug) {

+1, thank you! (I'll amend before merge or in the next version)

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 0/3] lib: LTP_DEBUG cleanup

[Andrea Cervesato via ltp]

Hi Petr,

Patches 1/3 and 2/3 look good. Patch 3/3 has a regression
and a couple of nits.

> [PATCH 3/3]
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> +	{"D::", "-D[1,2]  Prints debug information (can be overwriten by LTP_DEBUG"},

Typo: "overwriten" -> "overwritten". Also missing closing ')'.

[...]

> 	case 'D':
> +		if (!getenv("LTP_DEBUG")) {

This undoes the empty-string fix from patch 1/3. When
LTP_DEBUG="" is set, getenv() returns "" (non-NULL), so -D is
skipped here. But in do_setup() the tdebug_env[0] != '\0'
check also skips it. Net result: LTP_DEBUG= ./test -D2
silently produces no debug output.

Should be something like:

	char *env = getenv("LTP_DEBUG");
	if (!env || !env[0]) {

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

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

Re: [LTP] [PATCH v2 0/3] lib: LTP_DEBUG cleanup

[Petr Vorel]

> Hi Petr,

> Patches 1/3 and 2/3 look good. Patch 3/3 has a regression
> and a couple of nits.

> > [PATCH 3/3]
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > +	{"D::", "-D[1,2]  Prints debug information (can be overwriten by LTP_DEBUG"},

> Typo: "overwriten" -> "overwritten". Also missing closing ')'.

> [...]

> > 	case 'D':
> > +		if (!getenv("LTP_DEBUG")) {

> This undoes the empty-string fix from patch 1/3. When
> LTP_DEBUG="" is set, getenv() returns "" (non-NULL), so -D is
> skipped here. But in do_setup() the tdebug_env[0] != '\0'
> check also skips it. Net result: LTP_DEBUG= ./test -D2
> silently produces no debug output.

Good point, thanks!

nit: better to discuss in patch itself, discussion on cover letter is somehow
hidden in patchwork.

Kind regards,
Petr

> Should be something like:

> 	char *env = getenv("LTP_DEBUG");
> 	if (!env || !env[0]) {

> Regards,

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

Re: [LTP] [PATCH v2 0/3] lib: LTP_DEBUG cleanup

[Petr Vorel]

> > Hi Petr,

> > Patches 1/3 and 2/3 look good. Patch 3/3 has a regression
> > and a couple of nits.

> > > [PATCH 3/3]
> > > --- a/lib/tst_test.c
> > > +++ b/lib/tst_test.c
> > > +	{"D::", "-D[1,2]  Prints debug information (can be overwriten by LTP_DEBUG"},

> > Typo: "overwriten" -> "overwritten". Also missing closing ')'.

> > [...]

> > > 	case 'D':
> > > +		if (!getenv("LTP_DEBUG")) {

> > This undoes the empty-string fix from patch 1/3. When
> > LTP_DEBUG="" is set, getenv() returns "" (non-NULL), so -D is
> > skipped here. But in do_setup() the tdebug_env[0] != '\0'
> > check also skips it. Net result: LTP_DEBUG= ./test -D2
> > silently produces no debug output.

> Good point, thanks!

Actually, IMHO you're wrong. 1st commit treat empty LTP_DEBUG
as if it were not set (FYI LTP_DEBUG= and LTP_DEBUG="" are the same in shell -
variable is defined but empty).

And this functionality is kept here deliberately. LTP_DEBUG is always stronger,
that's why it ignores -D.

And why should be variable stronger? Can be set by user of any software. That's
actually the goal of environment variables. So we can run test via kirk/openQA
with LTP_DEBUG=2 to have different behavior without modifying software. But we
can also disable -D settings that way (imagine somebody puts -D1 into their
runner, user of that runner should be able to disable it). I'll describe it in
the commit message to be obvious.

Kind regards,
Petr

> nit: better to discuss in patch itself, discussion on cover letter is somehow
> hidden in patchwork.

> Kind regards,
> Petr

> > Should be something like:

> > 	char *env = getenv("LTP_DEBUG");
> > 	if (!env || !env[0]) {

> > Regards,

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