[LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

Petr Vorel (3):
  lib: Move IPC_ENV_VAR definition into header
  C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
  shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH

 doc/c-test-api.txt                          |  2 +-
 include/tst_test.h                          |  4 +++-
 lib/tst_test.c                              |  2 --
 testcases/kernel/syscalls/execle/execle01.c |  2 --
 testcases/kernel/syscalls/execve/execve01.c |  2 --
 testcases/kernel/syscalls/prctl/prctl06.h   |  1 -
 testcases/lib/tst_test.sh                   | 12 ++++++------
 7 files changed, 10 insertions(+), 15 deletions(-)

-- 
2.32.0

[LTP] [PATCH 1/3] lib: Move IPC_ENV_VAR definition into header

[Petr Vorel]

thus it can be used in tests instead of duplicate definition.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/tst_test.h                          | 2 ++
 lib/tst_test.c                              | 2 --
 testcases/kernel/syscalls/execle/execle01.c | 2 --
 testcases/kernel/syscalls/execve/execve01.c | 2 --
 testcases/kernel/syscalls/prctl/prctl06.h   | 1 -
 5 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index ce4e007cf..27ebed94e 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -275,6 +275,8 @@ struct tst_test {
 void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
                     __attribute__ ((noreturn));
 
+#define IPC_ENV_VAR "LTP_IPC_PATH"
+
 /*
  * Does library initialization for child processes started by exec()
  *
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 1bdea769a..b61aa8b03 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -71,8 +71,6 @@ static int ipc_fd;
 extern void *tst_futexes;
 extern unsigned int tst_max_futexes;
 
-#define IPC_ENV_VAR "LTP_IPC_PATH"
-
 static char ipc_path[1064];
 const char *tst_ipc_path = ipc_path;
 
diff --git a/testcases/kernel/syscalls/execle/execle01.c b/testcases/kernel/syscalls/execle/execle01.c
index f10813cca..917dc892f 100644
--- a/testcases/kernel/syscalls/execle/execle01.c
+++ b/testcases/kernel/syscalls/execle/execle01.c
@@ -16,8 +16,6 @@
 
 #include "tst_test.h"
 
-#define IPC_ENV_VAR "LTP_IPC_PATH"
-
 static void verify_execle(void)
 {
 	pid_t pid;
diff --git a/testcases/kernel/syscalls/execve/execve01.c b/testcases/kernel/syscalls/execve/execve01.c
index 9331c9480..2b12c7666 100644
--- a/testcases/kernel/syscalls/execve/execve01.c
+++ b/testcases/kernel/syscalls/execve/execve01.c
@@ -17,8 +17,6 @@
 
 #include "tst_test.h"
 
-#define IPC_ENV_VAR "LTP_IPC_PATH"
-
 static void verify_execve(void)
 {
 	pid_t pid;
diff --git a/testcases/kernel/syscalls/prctl/prctl06.h b/testcases/kernel/syscalls/prctl/prctl06.h
index 227ce3006..510fefa60 100644
--- a/testcases/kernel/syscalls/prctl/prctl06.h
+++ b/testcases/kernel/syscalls/prctl/prctl06.h
@@ -17,7 +17,6 @@
 #include "tst_test.h"
 
 #define PROC_STATUS        "/proc/self/status"
-#define IPC_ENV_VAR        "LTP_IPC_PATH"
 #define MNTPOINT           "mntpoint"
 #define TESTBIN            "prctl06_execve"
 #define TEST_REL_BIN_DIR   MNTPOINT"/"
-- 
2.32.0

[LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

To follow the conventions in the new API.

Update also C API docs.

NOTE: IPC is not supported in the legacy API.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/c-test-api.txt | 2 +-
 include/tst_test.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 3127018a4..a092bc572 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -654,7 +654,7 @@ int main(void)
 
 The 'tst_res()' function can be also used from binaries started by 'exec()',
 the parent test process has to set the '.child_needs_reinit' flag so that the
-library prepares for it and has to make sure the 'LTP_IPC_PATH' environment
+library prepares for it and has to make sure the 'TST_IPC_PATH' environment
 variable is passed down, then the very fist thing the program has to call in
 'main()' is 'tst_reinit()' that sets up the IPC.
 
diff --git a/include/tst_test.h b/include/tst_test.h
index 27ebed94e..4c0749865 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -275,12 +275,12 @@ struct tst_test {
 void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
                     __attribute__ ((noreturn));
 
-#define IPC_ENV_VAR "LTP_IPC_PATH"
+#define IPC_ENV_VAR "TST_IPC_PATH"
 
 /*
  * Does library initialization for child processes started by exec()
  *
- * The LTP_IPC_PATH variable must be passed to the program environment.
+ * The TST_IPC_PATH variable must be passed to the program environment.
  */
 void tst_reinit(void);
 
-- 
2.32.0

[LTP] [PATCH 3/3] shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

To follow the conventions in the new API.

Keep the old name in the legacy API.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_test.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index acf62c9ac..b68fae0b8 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -49,8 +49,8 @@ _tst_do_exit()
 		[ "$TST_TMPDIR_RHOST" = 1 ] && tst_cleanup_rhost
 	fi
 
-	if [ -n "$TST_NEEDS_CHECKPOINTS" -a -f "$LTP_IPC_PATH" ]; then
-		rm $LTP_IPC_PATH
+	if [ -n "$TST_NEEDS_CHECKPOINTS" -a -f "$TST_IPC_PATH" ]; then
+		rm $TST_IPC_PATH
 	fi
 
 	_tst_cleanup_timer
@@ -582,14 +582,14 @@ _tst_init_checkpoints()
 {
 	local pagesize
 
-	LTP_IPC_PATH="/dev/shm/ltp_${TST_ID}_$$"
+	TST_IPC_PATH="/dev/shm/ltp_${TST_ID}_$$"
 	pagesize=$(tst_getconf PAGESIZE)
 	if [ $? -ne 0 ]; then
 		tst_brk TBROK "tst_getconf PAGESIZE failed"
 	fi
-	ROD_SILENT dd if=/dev/zero of="$LTP_IPC_PATH" bs="$pagesize" count=1
-	ROD_SILENT chmod 600 "$LTP_IPC_PATH"
-	export LTP_IPC_PATH
+	ROD_SILENT dd if=/dev/zero of="$TST_IPC_PATH" bs="$pagesize" count=1
+	ROD_SILENT chmod 600 "$TST_IPC_PATH"
+	export TST_IPC_PATH
 }
 
 tst_run()
-- 
2.32.0

[LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Joerg Vehlow]

Hi Petr,

On 8/25/2021 10:09 AM, Petr Vorel wrote:
> To follow the conventions in the new API.
>
> Update also C API docs.
>
> NOTE: IPC is not supported in the legacy API.
Do you mean legacy c api here? It is supported. See old_checkpoints.h.

Joerg

[LTP] [PATCH 3/3] shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Joerg Vehlow]

Hi,

On 8/25/2021 10:09 AM, Petr Vorel wrote:
> To follow the conventions in the new API.
>
> Keep the old name in the legacy API.
That doesn't make sense. If you update the variable name in the c api, 
you also have to update the old shell api (i.e. test.sh).
Or maybe even remove the code from the old shell api completely. As far 
as I see there are no users of the checkpoint api anymore.
Both tests (cn_pec and memcfg/functional) were rewritten. It is probably 
safe to just drop the old api?

Joerg

[LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

Hi Joerg,

...
> > NOTE: IPC is not supported in the legacy API.
> Do you mean legacy c api here? It is supported. See old_checkpoints.h.
Ah good, point, thanks! (include/old/old_checkpoint.h). But if I look correctly,
LTP_IPC_PATH is not used there, right?

Kind regards,
Petr

[LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Cyril Hrubis]

Hi!
> Petr Vorel (3):
>   lib: Move IPC_ENV_VAR definition into header
>   C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
>   shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH

Is this really a good idea?

As it is it's pretty clear where the environment variable comes from, if
we rename it to TST_IPC_PATH it's not obvious that this has been
exported by LTP test.

Generally things that are visible on the running system tends to be
prefixed with LTP_ or ltp_ rather than TST_ or tst_...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

Hi Joerg,

> > Keep the old name in the legacy API.
> That doesn't make sense. If you update the variable name in the c api, you
> also have to update the old shell api (i.e. test.sh).
Ah, shell API implementation obviously depends on C API. Thanks for info.

> Or maybe even remove the code from the old shell api completely. As far as I
> see there are no users of the checkpoint api anymore.
> Both tests (cn_pec and memcfg/functional) were rewritten. It is probably
> safe to just drop the old api?
I'll check it, if it's really not used in the legacy API, I'd be also for removing it.


Kind regards,
Petr

[LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Joerg Vehlow]

Hi Petr,

On 8/25/2021 11:00 AM, Petr Vorel wrote:
> Hi Joerg,
>
> ...
>>> NOTE: IPC is not supported in the legacy API.
>> Do you mean legacy c api here? It is supported. See old_checkpoints.h.
> Ah good, point, thanks! (include/old/old_checkpoint.h). But if I look correctly,
> LTP_IPC_PATH is not used there, right?
It uses the same code as the new implementation (tst_checkpoint.c). So 
no changes necessary,
but the commit message is misleading.

Joerg

[LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

> Hi!
> > Petr Vorel (3):
> >   lib: Move IPC_ENV_VAR definition into header
> >   C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
> >   shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH

> Is this really a good idea?

> As it is it's pretty clear where the environment variable comes from, if
> we rename it to TST_IPC_PATH it's not obvious that this has been
> exported by LTP test.
I was thinking about it as well (just forget to write that into cover letter).

> Generally things that are visible on the running system tends to be
> prefixed with LTP_ or ltp_ rather than TST_ or tst_...
Sure. I just thought that LTP_ is for variables which are expected to be set by
user. But let's keep the old name. How about the first commit (cleanup)?

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/wiki/User-Guidelines

[LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Cyril Hrubis]

Hi!
> > As it is it's pretty clear where the environment variable comes from, if
> > we rename it to TST_IPC_PATH it's not obvious that this has been
> > exported by LTP test.
> I was thinking about it as well (just forget to write that into cover letter).
> 
> > Generally things that are visible on the running system tends to be
> > prefixed with LTP_ or ltp_ rather than TST_ or tst_...
> Sure. I just thought that LTP_ is for variables which are expected to be set by
> user. But let's keep the old name. How about the first commit (cleanup)?

There is no such rule I guess.

It has been case by case, for instance we do have KCONFIG_PATH which we
agreed on with the testing community to be a canonical variable name
that is shared between different frameworks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/3] lib: Move IPC_ENV_VAR definition into header

[Cyril Hrubis]

Hi!
This one is obviously okay.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/3] lib: Move IPC_ENV_VAR definition into header

[Petr Vorel]

Hi Cyril,

> Hi!
> This one is obviously okay.
Thanks, merged.

Kind regards,
Petr

[LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH

[Petr Vorel]

Hi Cyril, Joerg,

> Hi!
> > > As it is it's pretty clear where the environment variable comes from, if
> > > we rename it to TST_IPC_PATH it's not obvious that this has been
> > > exported by LTP test.
> > I was thinking about it as well (just forget to write that into cover letter).

> > > Generally things that are visible on the running system tends to be
> > > prefixed with LTP_ or ltp_ rather than TST_ or tst_...
> > Sure. I just thought that LTP_ is for variables which are expected to be set by
> > user. But let's keep the old name. How about the first commit (cleanup)?

> There is no such rule I guess.

> It has been case by case, for instance we do have KCONFIG_PATH which we
> agreed on with the testing community to be a canonical variable name
> that is shared between different frameworks.

Ah, thanks for info. Obviously there cannot be rules for everything :).
Thanks both for your time.

Kind regards,
Petr