Re: [LTP] [PATCH v3 3/8] syscalls/mknod03: Convert to new API

["Ricardo B. Marlière via ltp" <ltp@lists.linux.it>]

On Tue Apr 22, 2025 at 11:02 AM -03, Petr Vorel wrote:
> Hi Ricardo,
>
>> +++ b/testcases/kernel/syscalls/mknod/mknod03.c
>
>> @@ -1,296 +1,64 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
> Text below shows "or (at your option) any later version" => it should be GPLv2+.
> // SPDX-License-Identifier: GPL-2.0-or-later
>
>>  /*
>> - *
>> - *   Copyright (c) International Business Machines  Corp., 2001
>> - *
>> - *   This program is free software;  you can redistribute it and/or modify
>> - *   it under the terms of the GNU General Public License as published by
>> - *   the Free Software Foundation; either version 2 of the License, or
>> - *   (at your option) any later version.
>> - *
>> - *   This program is distributed in the hope that it will be useful,
>> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
>> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
>> - *   the GNU General Public License for more details.
>> - *
>> - *   You should have received a copy of the GNU General Public License
>> - *   along with this program;  if not, write to the Free Software
>> - *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + * Copyright (c) International Business Machines Corp., 2001
>> + *	07/2001 Ported by Wayne Boyer
>> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière <rbm@suse.com>
>>   */
>
> ...
>> +/*\
>> + * Verify that mknod(2) succeeds when used to create a filesystem node with
>> + * set-group-ID bit set on a directory with set-group-ID bit set. The node
>> + * created should have set-group-ID bit set and its gid should be equal to
>> + * the "nobody" gid.
>>   */
> ...
>> -#define MODE_RWX	S_IFIFO | S_IRWXU | S_IRWXG | S_IRWXO
>> -#define MODE_SGID       S_IFIFO | S_ISGID | S_IRWXU | S_IRWXG | S_IRWXO
>> -#define DIR_TEMP	"testdir_3"
>> -#define TNODE		"tnode_%d"
> ...
>> +#define MODE_RWX 0777
>> +#define MODE_SGID 2777
>
> The test fails on my VM (last minute change?):

Oof, yes that was indeed the last part I touched...

> mknod03.c:33: TFAIL: buf.st_mode & S_ISGID (0) != S_ISGID (1024)
> mknod03.c:38: TFAIL: buf.st_gid (65534) != free_gid (4)
>
> Why? S_ISGID is *not* 2000 but 02000 (octal number, 1535 in decimal).
> I actually find (S_ISGID | 0777) you had in v2 more readable, but maybe it's
> just me who needs to look up setuid bits (unlike typical rwx permission bits).
>

Agreed, I'll keep the octals only for permission modes. Good call :)

>> -char *TCID = "mknod03";
>> -int TST_TOTAL = 1;
>> -char node_name[PATH_MAX];	/* buffer to hold node name created */
>> +#define TEMP_DIR "testdir"
>> +#define TEMP_NODE TEMP_DIR "/testnode"
>
> ...
>> +static void run(void)
>>  {
> ...
>> +	struct stat buf;
>
>> +	SAFE_STAT(TEMP_DIR, &buf);
> I would keep this first SAFE_STAT() in the setup() as it was in the original
> code. We put things in the setup which are not related to the testing and thus
> it's a waste of time to repeat them, e.g. on repeated run:
> # ./mknod03 -i2000
>

Ack.

>> +	TST_EXP_EQ_LI(buf.st_mode & S_ISGID, S_ISGID);
> Also this original test should quit - use tst_brk(TBROK).
>
> NOTE: after Cyril's change [1] is merged, it will not need to be tst_brk(TBROK),
> but it makes sense to use TBROK (instead of TFAIL) in the test setup anyway.

I'll take a look!

>
> The rest LGTM.
>

Thanks for reviewing,
-	Ricardo.


> Kind regards,
> Petr
>
> [1] https://patchwork.ozlabs.org/project/ltp/patch/20250404143423.16268-1-chrubis@suse.cz/


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