[LTP] [PATCH] waitpid04: Convert to new API

[Martin Doucha]

On 31. 01. 24 18:21, Petr Vorel wrote:
>> Convert waitpid() error state checks to new API, fix bugs in the original
>> test and add ESRCH subtest.
> 
> Very nice cleanup.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> BTW what were these bugs? It looks like old test uses pid O for the first ECHILD
> test (not sure how it got defined to 0).

The first subtest used uninitialized pid value in the first iteration 
and then it'd test only pid=1 in all other iterations.

> 
>>   #include <sys/signal.h>
> nit: this should be <signal.h>
> warning: #warning redirecting incorrect #include <sys/signal.h> to <signal.h> [-Wcpp]
> 
> Can be fixed before merge.

Yes, please fix. Strangely, I didn't get this warning during compilation.

>>   #include <sys/types.h>
>>   #include <sys/wait.h>
> ...
>> +#include "tst_test.h"
>> +
>> +#define TCFMT "waipid(%d, NULL, 0x%x)"
>> +#define TCFMTARGS(tc) (tc)->pid, (tc)->flags
> 
> checkpatch.pl complains:
> waitpid04.c:19: ERROR: Macros with complex values should be enclosed in parentheses
> 
> I guess we can ignore this unless you see a quick fix (I guess macro which runs
> tst_res() inside would be needed.

The warning is expected, the macro should expand to multiple arguments. 
Parentheses would prevent that. I could fix it by tst_res(TINFO, 
"waitpid($args)"); before each test.

> 
>> +static struct testcase {
>> +	pid_t pid;
>> +	int flags;
>> +	int err;
>> +} testcase_list[] = {
>> +	{-1, 0, ECHILD},	/* Wait for any child when none exist */
>> +	{1, 0, ECHILD},		/* Wait for non-child process */
>> +	{-1, -1, EINVAL},	/* Invalid flags */
>> +	{INT_MIN, 0, ESRCH},	/* Wait for invalid process group */
>> +};
>> +
>> +void run(unsigned int n)
>>   {
> ...
>> +	const struct testcase *tc = testcase_list + n;
> 
>> -		if (FORK_OR_VFORK() == 0)
>> -			exit(0);
>> +	TEST(waitpid(tc->pid, NULL, tc->flags));
> How about using TST_EXP_FAIL2() to avoid code below? It would also help to avoid
> macros, right?  Or you want to explicitly state what failed?

The waitpid() call would be converted to a string that gives no useful 
information about which testcase is actually running and there isn't any 
good error message if the call succeeds.

> 
> Kind regards,
> Petr
> 
>> +	if (TST_RET == -1 && TST_ERR == tc->err) {
>> +		tst_res(TPASS | TTERRNO, TCFMT " failed as expected",
>> +			TCFMTARGS(tc));
>> +		return;
>> +	}
> 
>> +	if (TST_RET == -1) {
>> +		tst_res(TFAIL | TTERRNO, TCFMT ": expected error %s, got",
>> +			TCFMTARGS(tc), tst_strerrno(tc->err));
>> +		return;
>>   	}
> 
>> +	if (TST_RET < 0) {
>> +		tst_res(TFAIL | TTERRNO, TCFMT ": invalid return value %ld",
>> +			TCFMTARGS(tc), TST_RET);
>> +		return;
>> +	}
> 
>> +	tst_res(TFAIL, TCFMT " returned unexpected PID %ld", TCFMTARGS(tc),
>> +		TST_RET);
>>   }

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic