* [LTP] [PATCH 1/3] lib: Move IPC_ENV_VAR definition into header
2021-08-25 8:09 [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
@ 2021-08-25 8:09 ` Petr Vorel
2021-08-25 9:21 ` Cyril Hrubis
2021-08-25 8:09 ` [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 8:09 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:09 [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
2021-08-25 8:09 ` [LTP] [PATCH 1/3] lib: Move IPC_ENV_VAR definition into header Petr Vorel
@ 2021-08-25 8:09 ` Petr Vorel
2021-08-25 8:15 ` Joerg Vehlow
2021-08-25 8:09 ` [LTP] [PATCH 3/3] shell " Petr Vorel
2021-08-25 9:00 ` [LTP] [PATCH 0/3] New " Cyril Hrubis
3 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 8:09 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:09 ` [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
@ 2021-08-25 8:15 ` Joerg Vehlow
2021-08-25 9:00 ` Petr Vorel
0 siblings, 1 reply; 15+ messages in thread
From: Joerg Vehlow @ 2021-08-25 8:15 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:15 ` Joerg Vehlow
@ 2021-08-25 9:00 ` Petr Vorel
2021-08-25 9:08 ` Joerg Vehlow
0 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 9:00 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 9:00 ` Petr Vorel
@ 2021-08-25 9:08 ` Joerg Vehlow
0 siblings, 0 replies; 15+ messages in thread
From: Joerg Vehlow @ 2021-08-25 9:08 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 3/3] shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:09 [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
2021-08-25 8:09 ` [LTP] [PATCH 1/3] lib: Move IPC_ENV_VAR definition into header Petr Vorel
2021-08-25 8:09 ` [LTP] [PATCH 2/3] C API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
@ 2021-08-25 8:09 ` Petr Vorel
2021-08-25 8:18 ` Joerg Vehlow
2021-08-25 9:00 ` [LTP] [PATCH 0/3] New " Cyril Hrubis
3 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 8:09 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [LTP] [PATCH 3/3] shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:09 ` [LTP] [PATCH 3/3] shell " Petr Vorel
@ 2021-08-25 8:18 ` Joerg Vehlow
2021-08-25 9:04 ` Petr Vorel
0 siblings, 1 reply; 15+ messages in thread
From: Joerg Vehlow @ 2021-08-25 8:18 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 3/3] shell API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:18 ` Joerg Vehlow
@ 2021-08-25 9:04 ` Petr Vorel
0 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 9:04 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 8:09 [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH Petr Vorel
` (2 preceding siblings ...)
2021-08-25 8:09 ` [LTP] [PATCH 3/3] shell " Petr Vorel
@ 2021-08-25 9:00 ` Cyril Hrubis
2021-08-25 9:09 ` Petr Vorel
3 siblings, 1 reply; 15+ messages in thread
From: Cyril Hrubis @ 2021-08-25 9:00 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 9:00 ` [LTP] [PATCH 0/3] New " Cyril Hrubis
@ 2021-08-25 9:09 ` Petr Vorel
2021-08-25 9:20 ` Cyril Hrubis
0 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 9:09 UTC (permalink / raw)
To: ltp
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 9:09 ` Petr Vorel
@ 2021-08-25 9:20 ` Cyril Hrubis
2021-08-25 10:23 ` Petr Vorel
0 siblings, 1 reply; 15+ messages in thread
From: Cyril Hrubis @ 2021-08-25 9:20 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 0/3] New API: Rename LTP_IPC_PATH -> TST_IPC_PATH
2021-08-25 9:20 ` Cyril Hrubis
@ 2021-08-25 10:23 ` Petr Vorel
0 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2021-08-25 10:23 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread