[LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg

DAN LI

author

2013-05-29 09:01 UTC #0 lore raw

PATCH 1/1 shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg

1. Remove useless comments

2. Revise code to follow ltp-code-style

3. Add SHM_NORESERVE to shmflg to test

Signed-off-by DAN LI <[email protected]>

---
 testcases/kernel/syscalls/ipc/shmget/shmget01.c | 87 +++++++------------------
 1 file changed, 22 insertions(+), 65 deletions(-)

testcases/kernel/syscalls/ipc/shmget/shmget01.c +22 ~~-65~~

index 38dbee7..c36fb8e 100644
@@ -1,20 +1,19 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   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 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.
  *
- *   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
+ * 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
  */
 
 /*

@@ -33,27 +32,12 @@
  *	  if doing functionality testing
  *		stat the shared memory resource
  *		check the size, creator pid and mode
- *	  	if correct,
+ *		if correct,
  *			issue a PASS message
  *		otherwise
  *			issue a FAIL message
  *	  else issue a PASS message
  *	call cleanup
- *
- * USAGE:  <for command-line>
- *  shmget01 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *	       -i n : Execute test n times.
- *	       -I x : Execute test for x seconds.
- *	       -P x : Pause for x seconds between iterations.
- *	       -t   : Turn on syscall timing.
- *
- * HISTORY
- *	03/2001 - Written by Wayne Boyer
- *
- * RESTRICTIONS
- *	none
  */
 
 #include "ipcshm.h"

@@ -61,31 +45,25 @@
 char *TCID = "shmget01";
 int TST_TOTAL = 1;
 
-int shm_id_1 = -1;
+static int shm_id_1 = -1;
 
-int main(int ac, char **av)
+int main(int argc, char *argv[])
 {
 	int lc;
 	char *msg;
 	struct shmid_ds buf;
 
-	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL) {
+	msg = parse_opts(argc, argv, NULL, NULL);
+	if (msg != NULL)
 		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
-	}
 
-	setup();		/* global setup */
-
-	/* The following loop checks looping state if -i option given */
+	setup();
 
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		/* reset tst_count in case we are looping */
 		tst_count = 0;
 
-		/*
-		 * Use TEST macro to make the call
-		 */
-
-		TEST(shmget(shmkey, SHM_SIZE, (IPC_CREAT | IPC_EXCL | SHM_RW)));
+		TEST(shmget(shmkey, SHM_SIZE,
+			    (IPC_CREAT | IPC_EXCL | SHM_RW | SHM_NORESERVE)));
 
 		if (TEST_RETURN == -1) {
 			tst_resm(TFAIL, "%s call failed - errno = %d : %s",

@@ -131,11 +109,10 @@ int main(int ac, char **av)
 		/*
 		 * clean up things in case we are looping
 		 */
-		if (shmctl(shm_id_1, IPC_RMID, NULL) == -1) {
+		if (shmctl(shm_id_1, IPC_RMID, NULL) == -1)
 			tst_resm(TBROK, "couldn't remove shared memory");
-		} else {
+		else
 			shm_id_1 = -1;
-		}
 	}
 
 	cleanup();

@@ -143,42 +120,22 @@ int main(int ac, char **av)
 	tst_exit();
 }
 
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
 void setup(void)
 {
-
 	tst_sig(NOFORK, DEF_HANDLER, cleanup);
 
 	TEST_PAUSE;
 
-	/*
-	 * Create a temporary directory and cd into it.
-	 * This helps to ensure that a unique msgkey is created.
-	 * See ../lib/libipc.c for more information.
-	 */
 	tst_tmpdir();
 
-	/* get an IPC resource key */
 	shmkey = getipckey();
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
 void cleanup(void)
 {
-	/* if it exists, remove the shared memory resource */
 	rm_shm(shm_id_1);
 
 	tst_rmdir();
 
-	/*
-	 * print timing stats if that option was specified.
-	 * print errno log if that option was specified.
-	 */
 	TEST_CLEANUP;
-
 }

Signature · 12 lines

1.8.1

------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Jan Stancek

2013-05-29 09:18 UTC #1 lore raw

----- Original Message -----

Quoted 11 lines

From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
1. Remove useless comments
2. Revise code to follow ltp-code-style
3. Add SHM_NORESERVE to shmflg to test

Can you add some description why you are adding this flag?

Quoted 3 lines

Signed-off-by DAN LI <[email protected]>
---

------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with <2% overhead Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

DAN LI

author

2013-05-29 10:28 UTC #2 lore raw

Quoted 8 lines · Jan Stancek

----- Original Message -----
Quoted 6 lines
From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg

Quoted 7 lines

Quoted 5 lines
1. Remove useless comments
2. Revise code to follow ltp-code-style
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?

Actually, we are trying to complete test cases and expand the test scope of LTP.

So, it is just for completeness of this case.

Regards,
DAN LI

Quoted 6 lines

Quoted 4 lines
Signed-off-by DAN LI <[email protected]>
---

------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with <2% overhead Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Jan Stancek

2013-05-29 10:44 UTC #3 lore raw

----- Original Message -----

Quoted 20 lines

From: "DAN LI" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 12:25:45 PM
Subject: Re: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
Quoted 13 lines · Jan Stancek
----- Original Message -----
Quoted 11 lines
From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
1. Remove useless comments
2. Revise code to follow ltp-code-style

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.

My concern here is:
So if we add this flag, we cover SHM_NORESERVE path.
Do we have a testcase which still covers current non-SHM_NORESERVE path?

Regards,
Jan

Quoted 13 lines

Regards,
DAN LI
Quoted 6 lines
Quoted 4 lines
Signed-off-by DAN LI <[email protected]>
---

------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with <2% overhead Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

DAN LI

author

2013-05-31 07:23 UTC #4 lore raw

Quoted 26 lines · Jan Stancek

----- Original Message -----
Quoted 24 lines
From: "DAN LI" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 12:25:45 PM
Subject: Re: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
Quoted 16 lines · Jan Stancek
----- Original Message -----
Quoted 7 lines
From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
Quoted 5 lines
1. Remove useless comments
2. Revise code to follow ltp-code-style
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?

Quoted 8 lines

Quoted 4 lines
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.
My concern here is:
So if we add this flag, we cover SHM_NORESERVE path.
Do we have a testcase which still covers current non-SHM_NORESERVE path?

Hi Jan,

Sorry for the late reply. :)

In my opinion, SHM_NORESERVE is an independent feature of others and should have no effect on them, so we do not need a non-SHM_NORESERVE path.
They are like two branches of a river, one branch can do nothing on the other one, but if either of them goes wrong, the river will get feedback.

And maybe, it's just what the author of this case considered since he tested SHM_RW(S_IRUSR | S_IWUSR) together with IPC_CREAT | IPC_EXCL.

Regards,
DAN LI

Quoted 4 lines

Regards,
Jan

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite It's a free troubleshooting tool designed for production Get down to code-level detail for bottlenecks, with <2% overhead. Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap2
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Jan Stancek

2013-06-03 20:47 UTC #5 lore raw

----- Original Message -----

Quoted 46 lines

From: "DAN LI" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "LTP list" <[email protected]>
Sent: Friday, 31 May, 2013 9:20:58 AM
Subject: Re: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
Quoted 28 lines · Jan Stancek
----- Original Message -----
Quoted 26 lines
From: "DAN LI" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 12:25:45 PM
Subject: Re: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
Quoted 17 lines · Jan Stancek
----- Original Message -----
Quoted 8 lines
From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE
to shmflg
Quoted 5 lines
1. Remove useless comments
2. Revise code to follow ltp-code-style
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?
Quoted 8 lines
Quoted 4 lines
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.
My concern here is:
So if we add this flag, we cover SHM_NORESERVE path.
Do we have a testcase which still covers current non-SHM_NORESERVE path?
Hi Jan,

Quoted 8 lines

Sorry for the late reply. :)
In my opinion, SHM_NORESERVE is an independent feature of others and should have
no effect on them, so we do not need a non-SHM_NORESERVE path. They are like two branches of a river, one branch can do nothing on the other one,
but if either of them goes wrong, the river will get feedback.

Presence of that flag has some effect, absence has different one. To follow up on river metaphore, Are crocodiles in this river?

Anyway, we cover that path in other testcases, so unless someone else objects, I'm OK with adding that flag.

Regards,
Jan

Quoted 13 lines

And maybe, it's just what the author of this case considered since he tested SHM_RW(S_IRUSR | S_IWUSR) together with IPC_CREAT | IPC_EXCL.
Regards,
DAN LI
Quoted 4 lines
Regards,
Jan

------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. A cloud service to automate IT design, transition and operations
2. Dashboards that offer high-level views of enterprise services
3. A single system of record for all IT processes
http://p.sf.net/sfu/servicenow-d2d-j
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Caspar Zhang

2013-06-04 07:37 UTC #6 lore raw

Quoted 24 lines · Jan Stancek

Quoted 16 lines
Quoted 16 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.
My concern here is:
So if we add this flag, we cover SHM_NORESERVE path.
Do we have a testcase which still covers current non-SHM_NORESERVE path?
Hi Jan,
Quoted 8 lines
Quoted 8 lines
Sorry for the late reply.:)
In my opinion, SHM_NORESERVE is an independent feature of others and should have
no effect on them, so we do not need a non-SHM_NORESERVE path. They are like two branches of a river, one branch can do nothing on the other one,
but if either of them goes wrong, the river will get feedback.

Quoted 6 lines

Presence of that flag has some effect, absence has different one. To follow up on river metaphore, Are crocodiles in this river?
Anyway, we cover that path in other testcases, so unless someone else objects, I'm OK with adding that flag.

Crocodile spotted:

in fact in this patch, SHM_NORESERVE doesn't get fully tested. from the man page:

SHM_NORESERVE (since Linux 2.6.15)
[snip]When swap space is not reserved one might get SIGSEGV
upon a write if no physical memory is available. [snip]

We could design such a new testcase if we really want to cover the SHM_NORESERVE branch of river (and the RESERVE branch as well) by set the flag and fill the memory, check the result to see if a SIGSEGV is triggered.

Thoughts?

Caspar

------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. A cloud service to automate IT design, transition and operations
2. Dashboards that offer high-level views of enterprise services
3. A single system of record for all IT processes
http://p.sf.net/sfu/servicenow-d2d-j
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

DAN LI

author

2013-06-04 09:49 UTC #7 lore raw

Quoted 36 lines · Caspar Zhang

Quoted 24 lines · Jan Stancek
Quoted 16 lines
Quoted 16 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.
My concern here is:
So if we add this flag, we cover SHM_NORESERVE path.
Do we have a testcase which still covers current non-SHM_NORESERVE path?
Hi Jan,
Quoted 8 lines
Quoted 8 lines
Sorry for the late reply.:)
In my opinion, SHM_NORESERVE is an independent feature of others and should have
no effect on them, so we do not need a non-SHM_NORESERVE path. They are like two branches of a river, one branch can do nothing on the other one,
but if either of them goes wrong, the river will get feedback.
Quoted 6 lines
Presence of that flag has some effect, absence has different one. To follow up on river metaphore, Are crocodiles in this river?
Anyway, we cover that path in other testcases, so unless someone else objects, I'm OK with adding that flag.
Crocodile spotted:
in fact in this patch, SHM_NORESERVE doesn't get fully tested. from the man page:

Quoted 7 lines

SHM_NORESERVE (since Linux 2.6.15)
[snip]When swap space is not reserved one might get SIGSEGV upon a write if no physical memory is available. [snip]
We could design such a new testcase if we really want to cover the SHM_NORESERVE branch of river (and the RESERVE branch as well) by set the flag and fill the memory, check the result to see if a SIGSEGV is triggered.
Thoughts?

Do you have any idea about how or what i can use to fill memory and keep away from the OOM-kill, when trigger this SIGSEGV? :)

Regards,
DAN LI

Quoted 3 lines

Caspar

------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. A cloud service to automate IT design, transition and operations
2. Dashboards that offer high-level views of enterprise services
3. A single system of record for all IT processes
http://p.sf.net/sfu/servicenow-d2d-j
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Caspar Zhang

2013-05-29 11:57 UTC #8 lore raw

In reply to DAN LI

Quoted 13 lines · DAN LI

Quoted 12 lines · Jan Stancek
----- Original Message -----
Quoted 10 lines
From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
1. Remove useless comments
2. Revise code to follow ltp-code-style

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
3. Add SHM_NORESERVE to shmflg to test
Can you add some description why you are adding this flag?
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.

Would it be better to split the code-style fix and the functional/feature part?

Caspar

------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with <2% overhead Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

DAN LI

author

2013-05-31 07:28 UTC #9 lore raw

Quoted 16 lines · Caspar Zhang

Quoted 15 lines · DAN LI
Quoted 14 lines · Jan Stancek
----- Original Message -----
Quoted 11 lines
From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Sent: Wednesday, 29 May, 2013 10:58:47 AM
Subject: [LTP] [PATCH] shmget/shmget01.c: cleanup and attach SHM_NORESERVE to shmflg
1. Remove useless comments
2. Revise code to follow ltp-code-style
3. Add SHM_NORESERVE to shmflg to test

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
Can you add some description why you are adding this flag?
Actually, we are trying to complete test cases and expand the test scope of LTP.
So, it is just for completeness of this case.
Would it be better to split the code-style fix and the functional/feature part?

Agree.

Coming V2 will make it.

Regards,
DAN LI

Quoted 3 lines

Caspar

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite It's a free troubleshooting tool designed for production Get down to code-level detail for bottlenecks, with <2% overhead. Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap2
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list