Re: [LTP] [PATCH v3 4/5] syscalls/clone06: Convert to new API

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> -	cleanup();
> -	tst_exit();
> +	parent_env = getenv("TERM") ? : "";

I guess that it would make more sense to create a new variable than
depend on anything that may or may not be present on the system.

What about we setenv a variable instead and use that for the test?

#define ENV_VAL "LTP test variable value"
#define ENV_ID "LTP_CLONE_TEST"

static void setup(void)
{
	int ret;

	ret = setenv(ENV_ID, ENV_VAL, 0)
	if (ret)
		tst_brk(TBROK | TERRNO, "setenv() failed");

}


> +	TST_EXP_VAL(strcmp(buff, parent_env), 0,
> +				"verify environment variables by child");

Also there is no need to propagate the value to the parent like this,
the child process can report the result (in the new library) as well, so
this really could be as simple as:

static int do_child(void *arg LTP_ATTRIBUTE_UNUSED)
{
	const char *env_val = getenv(ENV_ID);

	if (!env_val) {
		tst_res(TFAIL, "Variable " ENV_ID " not defined in child");
		return;
	}

	if (strcmp(env_val, ENV_VAL)) {
		tst_res(TFAIL, "Variable value is different");
		return;
	}

	tst_res(TPASS, ...);
}

>  }
> 
>  static void setup(void)
>  {
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -	TEST_PAUSE;
> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> +	buff = SAFE_MMAP(NULL, MAX_LINE_LENGTH, PROT_READ | PROT_WRITE,
> +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>  }
> 
>  static void cleanup(void)
>  {
> -}
> -
> -/*
> - *	Function executed by child.
> - *	Gets the value for environment variable,TERM &
> - *	writes it to  a pipe.
> - */
> -static int child_environ(void)
> -{
> -
> -	char var[MAX_LINE_LENGTH];
> -
> -	/* Close read end from child */
> -	if ((close(pfd[0])) == -1)
> -		tst_brkm(TBROK | TERRNO, cleanup, "close(pfd[0]) failed");
> -
> -	if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0)
> -		tst_resm(TWARN | TERRNO, "sprintf() failed");
> -
> -	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1)
> -		tst_resm(TWARN | TERRNO, "write to pipe failed");
> -
> -	/* Close write end of pipe from child */
> -	if ((close(pfd[1])) == -1)
> -		tst_resm(TWARN | TERRNO, "close(pfd[1]) failed");
> +	free(child_stack);
> 
> -	exit(0);
> +	if (buff)
> +		SAFE_MUNMAP(buff, MAX_LINE_LENGTH);
>  }
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = verify_clone,
> +	.cleanup = cleanup,
> +};
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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