Re: [LTP] [PATCH] mem: Convert shmt02 to new API

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

Hi Andrea,

Thanks for the review, I'll address your points in v2. One comment
below.

On Wed Jun 18, 2025 at 9:44 AM -03, Andrea Cervesato wrote:
> Hi!
>
> On 6/18/25 13:46, Ricardo B. Marlière via ltp wrote:
>> From: Ricardo B. Marlière <rbm@suse.com>
>>
>> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
>> ---
>>   testcases/kernel/mem/shmt/shmt02.c | 118 +++++++------------------------------
>>   1 file changed, 21 insertions(+), 97 deletions(-)
>>
>> diff --git a/testcases/kernel/mem/shmt/shmt02.c b/testcases/kernel/mem/shmt/shmt02.c
>> index b33a042f93625cf44af1c3de6354248919f21522..f1dfec5028afb116f5393ff4b4869a7ea3099071 100644
>> --- a/testcases/kernel/mem/shmt/shmt02.c
>> +++ b/testcases/kernel/mem/shmt/shmt02.c
>> @@ -1,120 +1,44 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>> - *
>> - *   Copyright (c) International Business Machines  Corp., 2002
>> - *
>> - *   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., 2002
>> + *	12/20/2002	Port to LTP	robbiew@us.ibm.com
>> + *	06/30/2001	Port to Linux	nsharoff@us.ibm.com
>> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière <rbm@suse.com>
>>    */
>>   
>> -/* 12/20/2002		 Port to LTP		 robbiew@us.ibm.com */
>> -/* 06/30/2001		 Port to Linux		 nsharoff@us.ibm.com */
>> -
>> -/*
>> - * NAME
>> - *		 shmt02
>> - *
>> - * CALLS
>> - *		 shmctl(2) shmget(2)
>> - *
>> - * ALGORITHM
>> +/*\
>>    * Create and attach a shared memory segment, write to it
>> - * and then remove it.		  Verify that the shared memory segment
>> + * and then remove it. Verify that the shared memory segment
>>    * is accessible as long as the process is still alive.
>> - *
>>    */
>>   
>> -#include <stdio.h>
>> -#include <sys/types.h>
>> -#include <sys/ipc.h>
>> -#include <sys/shm.h>
>> -#include <sys/utsname.h>
>> -#include <errno.h>
>> -
>> -/** LTP Port **/
>> -#include "test.h"
>> -
>> -char *TCID = "shmt02";		/* Test program identifier.    */
>> -int TST_TOTAL = 3;		/* Total number of test cases. */
>> -
>> -/**************/
>> +#include "tst_safe_sysv_ipc.h"
> tst_test.h usually go first, because other headers often needs it.
>> +#include "tst_test.h"
>>   
>> -#define K_1 1024
>> -
>> -static int rm_shm(int);
>> -
>> -int main(void)
>> +static void run(void)
>>   {
>>   	register int shmid;
>>   	char *cp;
>>   	key_t key;
>>   
>>   	errno = 0;
>> -	key = (key_t) getpid();
>> -
>> -/*----------------------------------------------------------------*/
>> -
>> -	if ((shmid = shmget(key, 16 * K_1, IPC_CREAT | 0666)) < 0) {
>> -		perror("shmget");
>> -		tst_brkm(TFAIL, NULL,
>> -			 "shmget Failed: shmid = %d, errno = %d",
>> -			 shmid, errno);
>> -	}
>> +	key = (key_t)getpid();
>>   
>> -	tst_resm(TPASS, "shmget");
>> -
>> -/*----------------------------------------------------------------*/
>> -
>> -	cp = shmat(shmid, NULL, 0);
>> -
>> -	if (cp == (char *)-1) {
>> -		perror("shmat");
>> -		tst_resm(TFAIL, "shmat Failed: shmid = %d, errno = %d",
>> -			 shmid, errno);
>> -		rm_shm(shmid);
>> -		tst_exit();
>> -	}
>> +	shmid = SAFE_SHMGET(key, 16 * 1024, IPC_CREAT | 0666);
>
> #define SHMSIZE (16 * TST_KB)
>
> SAFE_SHMGET(key, SHMSIZE, IPC_CREAT | 0666);
>
> But I guess we can use a small buffer (read below). Everything will be 
> rounded to the next PAGE_SIZE anyway.
>
>>   
>> +	cp = SAFE_SHMAT(shmid, NULL, 0);
>>   	*cp = '1';
>>   	*(cp + 1) = '2';
>
> I'm wondering if it makes sense to use 2 bytes. Either we use one byte 
> or we write a random string and read it back. In the second case, we can 
> copy the first bytes of tst_rand_data bytes and compare them later on, 
> once  IPC_RMID is called.
>
>>   
>> -	tst_resm(TPASS, "shmat");
>> -
>> -/*----------------------------------------------------------------*/
>> -
>> -	rm_shm(shmid);
>> +	SAFE_SHMCTL(shmid, IPC_RMID, NULL);
>>   
>> -	if (*cp != '1' || *(cp + 1) != '2') {
>> -		tst_resm(TFAIL,
>> -			 "Error in shared memory contents: shmid = %d",
>> -			 shmid);
>> -	}
>> +	if (*cp != '1' || *(cp + 1) != '2')
>> +		tst_res(TFAIL, "Error in shared memory contents: shmid = %d",
>> +			shmid);
> Here we can just use TST_EXP_EQ_STRN if we copy random data or a simple 
> if statement for single byte.
>>   
>> -	tst_resm(TPASS, "Correct shared memory contents");
>> -
>> -/*------------------------------------------------------------------*/
>> -
>> -	tst_exit();
>> +	tst_res(TPASS, "Correct shared memory contents");
> This one should be inside the "else" statement.
>>   }
>>   
>> -static int rm_shm(int shmid)
>> -{
>> -	if (shmctl(shmid, IPC_RMID, NULL) == -1) {
>> -		perror("shmctl");
>> -		tst_brkm(TFAIL,
>> -			 NULL,
>> -			 "shmctl Failed to remove: shmid = %d, errno = %d",
>> -			 shmid, errno);
>> -	}
>> -	return (0);
>> -}
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +};
>>
>> ---
>> base-commit: df591113afeb31107bc45bd5ba28a99b556d1028
>> change-id: 20250618-conversions-shmt-5ea5a5ebe55e
>>
>> Best regards,
>
> We are only testing that the memory is accessible from the current 
> process, but we are not testing if the memory is no longer accessible 
> after the process which has created memory is still alive. This would 
> require to spawn a new process, allocate memory, check memory is 
> available after IPC_RMID, but not available after process exited.

IIUC, this scenario is covered in shmt04. So I'll keep this one simple
and address the other points, ok?

Thanks,
-	Ricardo.


>
> - Andrea


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