All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [RFC PATCH] tst_get_unused_pid and system_specific_process_info
@ 2014-06-25 10:00 Stanislav Kholmanskikh
  2014-06-25 12:14 ` chrubis
  0 siblings, 1 reply; 3+ messages in thread
From: Stanislav Kholmanskikh @ 2014-06-25 10:00 UTC (permalink / raw)
  To: ltp-list; +Cc: vasily.isaenko

Hi!

1. We remove get_max_pids(), because it's used only in one place (get_free_pids())
   and can be subsituted with
   SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &pid);

2. We rename get_free_pids() to tst_get_free_pids() for consistency. And changing a bit its
   declaration to use SAFE_FILE_SCANF().

3. system_specific_process_info.h is used only in one place - msgctl11.
   I think it's not a problem if I move its content to test.h. For consistency.

4. And add tst_get_unused_pid() to system_specific_process_info.c. To not
   create a new source file. And this funcion looks logically coupled with tst_get_free_pids().

I'm going to present pp. 1-3 in one patch, and p. 4 - in a separate patch.

What do you think about the general idea?

Thanks.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/system_specific_process_info.h          |   29 -------------
 include/test.h                                  |   15 +++++++
 lib/system_specific_process_info.c              |   49 ++++++----------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl11.c |    3 +-
 4 files changed, 30 insertions(+), 66 deletions(-)
 delete mode 100644 include/system_specific_process_info.h

diff --git a/include/system_specific_process_info.h b/include/system_specific_process_info.h
deleted file mode 100644
index ba8cc9d..0000000
--- a/include/system_specific_process_info.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- *
- *   Copyright (c) International Business Machines  Corp., 2009
- *
- *   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
- */
-
-#ifndef _SYS_SPECIFIC_PROCESS_INFO_H_
-#define _SYS_SPECIFIC_PROCESS_INFO_H_
-
-/* Returns max pid count obtained from reading /proc/sys/kernel/pid_max */
-int get_max_pids(void);
-
-/* Returns number of free pids */
-int get_free_pids(void);
-
-#endif
diff --git a/include/test.h b/include/test.h
index 97ce0f0..bf6deed 100644
--- a/include/test.h
+++ b/include/test.h
@@ -390,6 +390,21 @@ int tst_fs_fill_hardlinks(void (*cleanup) (void), const char *dir);
  */
 int tst_fs_fill_subdirs(void (*cleanup) (void), const char *dir);
 
+/*
+ * lib/system_specific_process_info.c
+ *
+ * Get a pid value not used by the OS
+ */
+pid_t tst_get_unused_pid(void (*cleanup_fn) (void));
+
+/*
+ * lib/system_specific_process_info.c
+ *
+ * Return a number of free pids by substracting the number
+ * of pid currently used ('ps -eT') from max_pids
+ */
+pid_t tst_get_free_pids(void (*cleanup_fn) (void));
+
 #ifdef TST_USE_COMPAT16_SYSCALL
 #define TCID_BIT_SUFFIX "_16"
 #elif  TST_USE_NEWER64_SYSCALL
diff --git a/lib/system_specific_process_info.c b/lib/system_specific_process_info.c
index bc47d49..105d806 100644
--- a/lib/system_specific_process_info.c
+++ b/lib/system_specific_process_info.c
@@ -1,6 +1,7 @@
 /*
  *
  *   Copyright (c) International Business Machines  Corp., 2009
+ *   Copyright (c) 2014 Oracle and/or its affiliates. All Rights Reserved.
  *
  *   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
@@ -17,50 +18,28 @@
  *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-/*
- *   DESCRIPTION
- *	get_max_pids(): Return the maximum number of pids for this system by
- *			reading /proc/sys/kernel/pid_max
- *
- *	get_free_pids(): Return number of free pids by subtracting the number
- *			 of pids currently used ('ps -eT') from max_pids
- */
-
 #include <fcntl.h>
 #include <limits.h>
 #include <sys/types.h>
 #include "test.h"
+#include "safe_file_ops.h"
 
-#define BUFSIZE 512
+#define PID_MAX_PATH "/proc/sys/kernel/pid_max"
 
-int get_max_pids(void)
+pid_t tst_get_unused_pid(void (*cleanup_fn) (void))
 {
-#ifdef __linux__
+	pid_t pid;
 
-	FILE *f;
-	char buf[BUFSIZE];
+	SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &pid);
 
-	f = fopen("/proc/sys/kernel/pid_max", "r");
-	if (!f) {
-		tst_resm(TBROK, "Could not open /proc/sys/kernel/pid_max");
-		return -1;
-	}
-	if (!fgets(buf, BUFSIZE, f)) {
-		fclose(f);
-		tst_resm(TBROK, "Could not read /proc/sys/kernel/pid_max");
-		return -1;
-	}
-	fclose(f);
-	return atoi(buf);
-#else
-	return SHRT_MAX;
-#endif
+	return pid;
 }
 
-int get_free_pids(void)
+pid_t tst_get_free_pids(void (*cleanup_fn) (void))
 {
 	FILE *f;
-	int rc, used_pids, max_pids;
+	int rc;
+	pid_t used_pids, max_pids;
 
 	f = popen("ps -eT | wc -l", "r");
 	if (!f) {
@@ -76,10 +55,10 @@ int get_free_pids(void)
 		return -1;
 	}
 
-	max_pids = get_max_pids();
-
-	if (max_pids < 0)
-		return -1;
+	SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &max_pids);
 
+	/* max_pids contains the maximum PID + 1,
+	 * used_pids contains used PIDs + 1,
+	 * so this additional '1' is eliminated by substraction*/
 	return max_pids - used_pids;
 }
diff --git a/testcases/kernel/syscalls/ipc/msgctl/msgctl11.c b/testcases/kernel/syscalls/ipc/msgctl/msgctl11.c
index 75d8bde..cffabf5 100644
--- a/testcases/kernel/syscalls/ipc/msgctl/msgctl11.c
+++ b/testcases/kernel/syscalls/ipc/msgctl/msgctl11.c
@@ -40,7 +40,6 @@
 #include "usctest.h"
 #include "ipcmsg.h"
 #include "../lib/libmsgctl.h"
-#include "system_specific_process_info.h"
 
 char *TCID = "msgctl11";
 int TST_TOTAL = 1;
@@ -475,7 +474,7 @@ void setup(void)
 
 	tst_resm(TINFO, "Found %d available message queues", MSGMNI);
 
-	free_pids = get_free_pids();
+	free_pids = tst_get_free_pids(cleanup);
 	if (free_pids < 0) {
 		tst_brkm(TBROK, cleanup, "Can't obtain free_pid count");
 	} else if (!free_pids) {
-- 
1.7.1


------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [LTP] [RFC PATCH] tst_get_unused_pid and system_specific_process_info
  2014-06-25 10:00 [LTP] [RFC PATCH] tst_get_unused_pid and system_specific_process_info Stanislav Kholmanskikh
@ 2014-06-25 12:14 ` chrubis
       [not found]   ` <53AABDA5.9010107@oracle.com>
  0 siblings, 1 reply; 3+ messages in thread
From: chrubis @ 2014-06-25 12:14 UTC (permalink / raw)
  To: Stanislav Kholmanskikh; +Cc: vasily.isaenko, ltp-list

Hi!
> 1. We remove get_max_pids(), because it's used only in one place (get_free_pids())
>    and can be subsituted with
>    SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &pid);
> 
> 2. We rename get_free_pids() to tst_get_free_pids() for consistency. And changing a bit its
>    declaration to use SAFE_FILE_SCANF().
> 
> 3. system_specific_process_info.h is used only in one place - msgctl11.
>    I think it's not a problem if I move its content to test.h. For consistency.
> 
> 4. And add tst_get_unused_pid() to system_specific_process_info.c. To not
>    create a new source file. And this funcion looks logically coupled with tst_get_free_pids().
> 
> I'm going to present pp. 1-3 in one patch, and p. 4 - in a separate patch.
> 
> What do you think about the general idea?

Sounds good to me.

Perhaps we should also rename the system_specific_process_info.c to
something shorter and more to the point. Maybe just process_info.c or
pidutils.c or something similar.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [LTP] [RFC PATCH] tst_get_unused_pid and system_specific_process_info
       [not found]   ` <53AABDA5.9010107@oracle.com>
@ 2014-06-25 12:18     ` chrubis
  0 siblings, 0 replies; 3+ messages in thread
From: chrubis @ 2014-06-25 12:18 UTC (permalink / raw)
  To: Stanislav Kholmanskikh; +Cc: vasily.isaenko, ltp-list

Hi!
> >> 1. We remove get_max_pids(), because it's used only in one place (get_free_pids())
> >>     and can be subsituted with
> >>     SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &pid);
> >>
> >> 2. We rename get_free_pids() to tst_get_free_pids() for consistency. And changing a bit its
> >>     declaration to use SAFE_FILE_SCANF().
> >>
> >> 3. system_specific_process_info.h is used only in one place - msgctl11.
> >>     I think it's not a problem if I move its content to test.h. For consistency.
> >>
> >> 4. And add tst_get_unused_pid() to system_specific_process_info.c. To not
> >>     create a new source file. And this funcion looks logically coupled with tst_get_free_pids().
> >>
> >> I'm going to present pp. 1-3 in one patch, and p. 4 - in a separate patch.
> >>
> >> What do you think about the general idea?
> >
> > Sounds good to me.
> >
> > Perhaps we should also rename the system_specific_process_info.c to
> > something shorter and more to the point. Maybe just process_info.c or
> > pidutils.c or something similar.
> >
> Maybe tst_pid.c ? :)

Sounds good.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-06-25 12:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-25 10:00 [LTP] [RFC PATCH] tst_get_unused_pid and system_specific_process_info Stanislav Kholmanskikh
2014-06-25 12:14 ` chrubis
     [not found]   ` <53AABDA5.9010107@oracle.com>
2014-06-25 12:18     ` chrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.