Re: [LTP] [PATCH v4] Rewrite userns06.c using new LTP API

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

[-- Attachment #1.1: Type: text/plain, Size: 2530 bytes --]

Hi Petr,

can TST_TEST_CONF be used even if TST_NO_DEFAULT_MAIN is defined?

Andrea

On 4/4/22 12:33, Petr Vorel wrote:
> Hi Andrea,
>
> BTW it'd help reviewers a bit if you include a changelog.
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
> TL;DR: TST_TEST_TCONF() in userns06_capcheck.c
>
>> diff --git a/testcases/kernel/containers/userns/userns06_capcheck.c b/testcases/kernel/containers/userns/userns06_capcheck.c
> ...
>> -/*
>> - * Verify that:
>> +/*\
>> + * [Description]
>> + *
>>    * When a process with non-zero user IDs performs an execve(), the
>>    * process's capability sets are cleared. When a process with zero
>>    * user IDs performs an execve(), the process's capability sets
>>    * are set.
>>    */
> nit: I wonder if we want to have docparse documentation in both userns06.c and
> userns06_capcheck.c, they now look as 2 separate tests. Maybe describe
> everything in userns06.c.
>
>> +#define TST_NO_DEFAULT_MAIN
>> +#include "tst_test.h"
>> +#include "config.h"
>> +
>> +#ifdef HAVE_LIBCAP
> ...
>> +	if (argc < 2)
>> +		tst_brk(TBROK, "userns06_capcheck <privileged|unprivileged>");
>> +
>> +	tst_reinit();
> I'm not sure if tst_reinit() shouldn't be called even before tst_brk(TBROK, ...).
>> +
>> +	SAFE_FILE_SCANF("/proc/sys/kernel/cap_last_cap", "%d", &last_cap);
>> +
>>   	if (strcmp("privileged", argv[1]))
>>   		expected_flag = 0;
> nit: It might help debugging to print argv[1] in TINF0.
>
>>   	caps = cap_get_proc();
>> -	SAFE_FILE_SCANF(NULL, "/proc/sys/kernel/cap_last_cap", "%d", &last_cap);
>> +
>>   	for (i = 0; i <= last_cap; i++) {
>>   		cap_get_flag(caps, i, CAP_EFFECTIVE, &flag_val);
>>   		if (flag_val != expected_flag)
>>   			break;
>> +
>>   		cap_get_flag(caps, i, CAP_PERMITTED, &flag_val);
>>   		if (flag_val != expected_flag)
>>   			break;
>>   	}
>> -	if (flag_val != expected_flag) {
>> -		printf("unexpected effective/permitted caps at %d\n", i);
>> -		exit(1);
>> -	}
>> +	if (flag_val != expected_flag)
>> +		tst_res(TFAIL, "unexpected effective/permitted caps at %d", i);
> The flags are CAP_EFFECTIVE and CAP_PERMITTED only here, right?
> (i.e. no CAP_INHERITABLE). Not sure how helpful would be to print here which
> flag was the failing one.
>
>> +	else
>> +		tst_res(TPASS, "expected caps at %d", i);
>> +}
>>   #else
>> -	printf("System is missing libcap.\n");
>> -#endif
>> -	tst_exit();
>> +int main(void)
>> +{
>> +	tst_brk(TBROK, "System is missing libcap");
>>   }
> Why don't you also use TST_TEST_TCONF() here?
>> +#endif
> Kind regards,
> Petr
>

[-- Attachment #1.2: Type: text/html, Size: 4424 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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