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

[zhanglianjie]

Hi,
This is a good idea, just do it, thank you.

On 2021-10-12 17:21, Cyril Hrubis wrote:
> 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
> 

-- 
Regards,
Zhang Lianjie