[LTP] [PATCH 1/6] lib: add LTP_IPC_PATH to env. variables by default

[LTP] [PATCH 1/6] lib: add LTP_IPC_PATH to env. variables by default

[Jan Stancek]

Child started by exec() that needs to do tst_reinit() in order
to use checkpoints will inherit LTP_IPC_PATH by default.
Parent can choose to override/omit it in envp array when calling
exec[lvp]e().

Also fix couple places to use LTP_IPC_PATH define.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 lib/tst_test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 26414e31ca77..25cb234425ab 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -99,10 +99,12 @@ static void setup_ipc(void)
 	results = SAFE_MMAP(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, ipc_fd, 0);
 
 	/* Checkpoints needs to be accessible from processes started by exec() */
-	if (tst_test->needs_checkpoints)
+	if (tst_test->needs_checkpoints) {
 		sprintf(ipc_path, IPC_ENV_VAR "=%s", shm_path);
-	else
+		putenv(ipc_path);
+	} else {
 		SAFE_UNLINK(shm_path);
+	}
 
 	SAFE_CLOSE(ipc_fd);
 
@@ -130,13 +132,13 @@ static void cleanup_ipc(void)
 
 void tst_reinit(void)
 {
-	const char *path = getenv("LTP_IPC_PATH");
+	const char *path = getenv(IPC_ENV_VAR);
 	size_t size = getpagesize();
 	int fd;
 	void *ptr;
 
 	if (!path)
-		tst_brk(TBROK, "LTP_IPC_PATH is not defined");
+		tst_brk(TBROK, IPC_ENV_VAR" is not defined");
 
 	if (access(path, F_OK))
 		tst_brk(TBROK, "File %s does not exist!", path);
-- 
1.8.3.1

[LTP] [PATCH 2/6] lib: move tst_get_startwd() to old_tmpdir header

[Jan Stancek]

Newlib can't include ltp_priv.h due to conflicts with oldlib.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/old/ltp_priv.h   | 3 ---
 include/old/old_tmpdir.h | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/old/ltp_priv.h b/include/old/ltp_priv.h
index e678a28682eb..0552457e591f 100644
--- a/include/old/ltp_priv.h
+++ b/include/old/ltp_priv.h
@@ -24,9 +24,6 @@
 
 #include <stdarg.h>
 
-/* declared in tst_tmpdir.c */
-const char *tst_get_startwd(void);
-
 /*
  * This is the default temporary directory used by tst_tmpdir().
  *
diff --git a/include/old/old_tmpdir.h b/include/old/old_tmpdir.h
index b39b615a8bdb..9c61172fd71c 100644
--- a/include/old/old_tmpdir.h
+++ b/include/old/old_tmpdir.h
@@ -50,4 +50,7 @@ char *tst_get_tmpdir(void);
  */
 int tst_tmpdir_created(void);
 
+/* declared in tst_tmpdir.c */
+const char *tst_get_startwd(void);
+
 #endif	/* OLD_TMPDIR_H__ */
-- 
1.8.3.1

[LTP] [PATCH 3/6] safe_macros: add SAFE_SETENV()

[Jan Stancek]

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/tst_safe_macros.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 1e034b8e29d7..5d8952544be3 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -447,5 +447,12 @@ int safe_personality(const char *filename, unsigned int lineno,
 		    unsigned long persona);
 #define SAFE_PERSONALITY(persona) safe_personality(__FILE__, __LINE__, persona)
 
+#define SAFE_SETENV(name, value, overwrite) do {		\
+	if (setenv(name, value, overwrite)) {			\
+		tst_brk_(__FILE__, __LINE__, TBROK | TERRNO,	\
+			"setenv(%s, %s, %d) failed",		\
+			name, value, overwrite);		\
+	}							\
+	} while (0)
 
 #endif /* SAFE_MACROS_H__ */
-- 
1.8.3.1

[LTP] [PATCH 4/6] lib: extend PATH as part of test setup

[Jan Stancek]

This patch adds "." and "startdir" to $PATH during setup, so that exec*p()
correctly finds child processes. And it also makes it convenient to run some
tests directly from git tree.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 lib/tst_test.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 25cb234425ab..78cce19b6e4a 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -841,12 +841,41 @@ static unsigned long long get_time_ms(void)
 	return tv.tv_sec * 1000 + tv.tv_usec / 1000;
 }
 
+static void add_paths(void)
+{
+	char *old_path = getenv("PATH");
+	const char *start_dir;
+	char *new_path;
+	int new_path_size;
+
+	start_dir = tst_get_startwd();
+
+	/* ".:" + "$start_dir" + '\0' */
+	new_path_size = 2 + strlen(start_dir) + 1;
+	if (old_path)
+		/* ":" + "$old_path" */
+		new_path_size += 1 + strlen(old_path);
+
+	new_path = SAFE_MALLOC(new_path_size);
+	strcpy(new_path, ".:");
+	strcat(new_path, start_dir);
+
+	if (old_path) {
+		strcat(new_path, ":");
+		strcat(new_path, old_path);
+	}
+
+	SAFE_SETENV("PATH", new_path, 1);
+	free(new_path);
+}
+
 static void testrun(void)
 {
 	unsigned int i = 0;
 	unsigned long long stop_time = 0;
 	int cont = 1;
 
+	add_paths();
 	do_test_setup();
 
 	if (duration > 0)
-- 
1.8.3.1

[LTP] [PATCH 5/6] creat07: no need to pass tst_ipc_envp anymore

[Jan Stancek]

Now that env. variable LTP_IPC_PATH is inherited by default,
it's not necessary to pass tst_ipc_envp anymore.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/creat/creat07.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/creat/creat07.c b/testcases/kernel/syscalls/creat/creat07.c
index e1b267661c39..236f4edf843f 100644
--- a/testcases/kernel/syscalls/creat/creat07.c
+++ b/testcases/kernel/syscalls/creat/creat07.c
@@ -38,9 +38,7 @@ static void verify_creat(void)
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
-		char *av[] = {TEST_APP, NULL};
-		(void)execve(TEST_APP, av, tst_ipc_envp);
-		perror("execve failed");
+		SAFE_EXECLP(TEST_APP, TEST_APP, NULL);
 		exit(1);
 	}
 
-- 
1.8.3.1

[LTP] [PATCH 6/6] lib: drop tst_ipc_envp

[Jan Stancek]

There are no users left.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/tst_checkpoint.h | 1 -
 lib/tst_test.c           | 1 -
 2 files changed, 2 deletions(-)

diff --git a/include/tst_checkpoint.h b/include/tst_checkpoint.h
index fdce2d6f70b6..d6f7607abade 100644
--- a/include/tst_checkpoint.h
+++ b/include/tst_checkpoint.h
@@ -37,6 +37,5 @@
         tst_safe_checkpoint_wait(__FILE__, __LINE__, NULL, id, 0);
 
 extern const char *tst_ipc_path;
-extern char *const tst_ipc_envp[];
 
 #endif /* TST_CHECKPOINT__ */
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 78cce19b6e4a..184a9f06aa72 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -64,7 +64,6 @@ extern unsigned int tst_max_futexes;
 
 static char ipc_path[1024];
 const char *tst_ipc_path = ipc_path;
-char *const tst_ipc_envp[] = {ipc_path, NULL};
 
 static char shm_path[1024];
 
-- 
1.8.3.1

[LTP] [PATCH 4/6] lib: extend PATH as part of test setup

[Cyril Hrubis]

Hi!
> +static void add_paths(void)
> +{
> +	char *old_path = getenv("PATH");
> +	const char *start_dir;
> +	char *new_path;
> +	int new_path_size;
> +
> +	start_dir = tst_get_startwd();
> +
> +	/* ".:" + "$start_dir" + '\0' */
> +	new_path_size = 2 + strlen(start_dir) + 1;
> +	if (old_path)
> +		/* ":" + "$old_path" */
> +		new_path_size += 1 + strlen(old_path);
> +
> +	new_path = SAFE_MALLOC(new_path_size);
> +	strcpy(new_path, ".:");
> +	strcat(new_path, start_dir);
> +
> +	if (old_path) {
> +		strcat(new_path, ":");
> +		strcat(new_path, old_path);
> +	}

I do not like the strcat() function much, I would have probably handled
these with asprintf();

	if (old_path)
		SAFE_ASPRINTF(&new_path, "%s::%s", old_path, start_dir);
	else
		SAFE_ASPRINTF(&new_path, "::%s", start_dir);


The rest of the patchset looks good.

BTW, we should also drop the note about setting $PATH from the
documentation once this is merged.

> +	SAFE_SETENV("PATH", new_path, 1);
> +	free(new_path);
> +}
> +
>  static void testrun(void)
>  {
>  	unsigned int i = 0;
>  	unsigned long long stop_time = 0;
>  	int cont = 1;
>  
> +	add_paths();
>  	do_test_setup();
>  
>  	if (duration > 0)
> -- 
> 1.8.3.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/6] lib: extend PATH as part of test setup

[Jan Stancek]

----- Original Message -----
> I do not like the strcat() function much, I would have probably handled
> these with asprintf();
> 
> 	if (old_path)
> 		SAFE_ASPRINTF(&new_path, "%s::%s", old_path, start_dir);
> 	else
> 		SAFE_ASPRINTF(&new_path, "::%s", start_dir);

This makes it simpler, I've modified it to use SAFE_ASPRINTF.

> 
> 
> The rest of the patchset looks good.

I pushed all with your ACK.

> 
> BTW, we should also drop the note about setting $PATH from the
> documentation once this is merged.

Or tweak it to say, it's no longer necessary for newlib tests.

Regards,
Jan

[LTP] [PATCH 4/6] lib: extend PATH as part of test setup

[Cyril Hrubis]

Hi!
> > BTW, we should also drop the note about setting $PATH from the
> > documentation once this is merged.
> 
> Or tweak it to say, it's no longer necessary for newlib tests.

Ah, right we still do have oldlib tests.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 5/6] creat07: no need to pass tst_ipc_envp anymore

[Cyril Hrubis]

Hi!
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  testcases/kernel/syscalls/creat/creat07.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/creat/creat07.c b/testcases/kernel/syscalls/creat/creat07.c
> index e1b267661c39..236f4edf843f 100644
> --- a/testcases/kernel/syscalls/creat/creat07.c
> +++ b/testcases/kernel/syscalls/creat/creat07.c
> @@ -38,9 +38,7 @@ static void verify_creat(void)
>  
>  	pid = SAFE_FORK();
>  	if (pid == 0) {
> -		char *av[] = {TEST_APP, NULL};
> -		(void)execve(TEST_APP, av, tst_ipc_envp);
> -		perror("execve failed");
> +		SAFE_EXECLP(TEST_APP, TEST_APP, NULL);

And it looks like this broke the test when executed from installed LTP,
that is because the execlp() picks up the TEST_APP from
/opt/ltp/testcasese/bin/ instead of the local copy. I guess that we have
to use exec() variant without the p at the end here.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 5/6] creat07: no need to pass tst_ipc_envp anymore

[Li Wang]

On Thu, Oct 12, 2017 at 4:08 PM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>> ---
>>  testcases/kernel/syscalls/creat/creat07.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/creat/creat07.c b/testcases/kernel/syscalls/creat/creat07.c
>> index e1b267661c39..236f4edf843f 100644
>> --- a/testcases/kernel/syscalls/creat/creat07.c
>> +++ b/testcases/kernel/syscalls/creat/creat07.c
>> @@ -38,9 +38,7 @@ static void verify_creat(void)
>>
>>       pid = SAFE_FORK();
>>       if (pid == 0) {
>> -             char *av[] = {TEST_APP, NULL};
>> -             (void)execve(TEST_APP, av, tst_ipc_envp);
>> -             perror("execve failed");
>> +             SAFE_EXECLP(TEST_APP, TEST_APP, NULL);
>
> And it looks like this broke the test when executed from installed LTP,
> that is because the execlp() picks up the TEST_APP from

Yes, I also hit the problem when running creat07 with LTP framework.

# /opt/ltp/runltp -s creat07
...
incrementing stop
tst_test.c:958: INFO: Timeout per run is 0h 05m 00s
creat07.c:50: FAIL: creat() succeeded unexpectedly
^Cincrementing stop
Sending SIGKILL to test process...
tst_test.c:1008: INFO: If you are running on slow machine, try
exporting LTP_TIMEOUT_MUL > 1
tst_test.c:1009: BROK: Test killed! (timeout?)


> /opt/ltp/testcasese/bin/ instead of the local copy. I guess that we have
> to use exec() variant without the p at the end here.

Right! After replacing the execlp() with execl(), the issue is gone.

# /opt/ltp/runltp -s creat07
...
incrementing stop
tst_test.c:958: INFO: Timeout per run is 0h 05m 00s
creat07.c:55: PASS: creat() received EXTBSY

Summary:
passed   1
failed   0
skipped  0
warnings 0


------------------
$ git diff
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 5d89525..a865cbd 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -403,6 +403,12 @@ static inline sighandler_t safe_signal(const char
*file, const int lineno,
                 "execlp(%s, %s, ...) failed", file, arg); \
        } while (0)

+#define SAFE_EXECL(file, arg, ...) do {                   \
+       execl((file), (arg), ##__VA_ARGS__);              \
+       tst_brk_(__FILE__, __LINE__, TBROK | TERRNO,       \
+                "execlp(%s, %s, ...) failed", file, arg); \
+       } while (0)
+
 int safe_getpriority(const char *file, const int lineno, int which, id_t who);
 #define SAFE_GETPRIORITY(which, who) \
        safe_getpriority(__FILE__, __LINE__, (which), (who))
diff --git a/testcases/kernel/syscalls/creat/creat07.c
b/testcases/kernel/syscalls/creat/creat07.c
index 236f4ed..a016604 100644
--- a/testcases/kernel/syscalls/creat/creat07.c
+++ b/testcases/kernel/syscalls/creat/creat07.c
@@ -38,7 +38,7 @@ static void verify_creat(void)

        pid = SAFE_FORK();
        if (pid == 0) {
-               SAFE_EXECLP(TEST_APP, TEST_APP, NULL);
+               SAFE_EXECL(TEST_APP, TEST_APP, NULL);
                exit(1);
        }





-- 
Li Wang
liwang@redhat.com

[LTP] [PATCH 5/6] creat07: no need to pass tst_ipc_envp anymore

[Jan Stancek]

----- Original Message -----
> Hi!
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >  testcases/kernel/syscalls/creat/creat07.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/testcases/kernel/syscalls/creat/creat07.c
> > b/testcases/kernel/syscalls/creat/creat07.c
> > index e1b267661c39..236f4edf843f 100644
> > --- a/testcases/kernel/syscalls/creat/creat07.c
> > +++ b/testcases/kernel/syscalls/creat/creat07.c
> > @@ -38,9 +38,7 @@ static void verify_creat(void)
> >  
> >  	pid = SAFE_FORK();
> >  	if (pid == 0) {
> > -		char *av[] = {TEST_APP, NULL};
> > -		(void)execve(TEST_APP, av, tst_ipc_envp);
> > -		perror("execve failed");
> > +		SAFE_EXECLP(TEST_APP, TEST_APP, NULL);
> 
> And it looks like this broke the test when executed from installed LTP,
> that is because the execlp() picks up the TEST_APP from
> /opt/ltp/testcasese/bin/ instead of the local copy. I guess that we have
> to use exec() variant without the p at the end here.

Or we can switch order so that "." is searched as first:
  SAFE_ASPRINTF(&new_path, ":%s:%s", start_dir, old_path);
What do you think?

Regards,
Jan

[LTP] [PATCH 5/6] creat07: no need to pass tst_ipc_envp anymore

[Jan Stancek]

----- Original Message -----
> From: "Jan Stancek" <jstancek@redhat.com>
> To: "Cyril Hrubis" <chrubis@suse.cz>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 12 October, 2017 10:50:32 AM
> Subject: Re: [LTP] [PATCH 5/6] creat07: no need to pass tst_ipc_envp anymore
> 
> 
> ----- Original Message -----
> > Hi!
> > > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > > ---
> > >  testcases/kernel/syscalls/creat/creat07.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/testcases/kernel/syscalls/creat/creat07.c
> > > b/testcases/kernel/syscalls/creat/creat07.c
> > > index e1b267661c39..236f4edf843f 100644
> > > --- a/testcases/kernel/syscalls/creat/creat07.c
> > > +++ b/testcases/kernel/syscalls/creat/creat07.c
> > > @@ -38,9 +38,7 @@ static void verify_creat(void)
> > >  
> > >  	pid = SAFE_FORK();
> > >  	if (pid == 0) {
> > > -		char *av[] = {TEST_APP, NULL};
> > > -		(void)execve(TEST_APP, av, tst_ipc_envp);
> > > -		perror("execve failed");
> > > +		SAFE_EXECLP(TEST_APP, TEST_APP, NULL);
> > 
> > And it looks like this broke the test when executed from installed LTP,
> > that is because the execlp() picks up the TEST_APP from
> > /opt/ltp/testcasese/bin/ instead of the local copy. I guess that we have
> > to use exec() variant without the p at the end here.
> 
> Or we can switch order so that "." is searched as first:
>   SAFE_ASPRINTF(&new_path, ":%s:%s", start_dir, old_path);
> What do you think?

I pushed Li's version, while this is being discussed.

Regards,
Jan