[PATCH 1/4] lib: add function to seed random()

[PATCH 1/4] lib: add function to seed random()

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 include/randutils.h |  1 +
 lib/randutils.c     | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/randutils.h b/include/randutils.h
index dec5e35..9d4d41c 100644
--- a/include/randutils.h
+++ b/include/randutils.h
@@ -8,5 +8,6 @@
 
 extern int random_get_fd(void);
 extern void random_get_bytes(void *buf, size_t nbytes);
+extern void seed_random(void);
 
 #endif
diff --git a/lib/randutils.c b/lib/randutils.c
index 85cb1a9..44d996e 100644
--- a/lib/randutils.c
+++ b/lib/randutils.c
@@ -104,6 +104,24 @@ void random_get_bytes(void *buf, size_t nbytes)
 	return;
 }
 
+/* Seed random(3). */
+void seed_random(void)
+{
+	struct timeval tv;
+	unsigned int seed;
+	FILE *fd;
+
+	if ((fd = fopen("/dev/urandom", "r"))) {
+		fread(&seed, sizeof seed, 1, fd);
+		fclose(fd);
+	} else {
+		gettimeofday(&tv, NULL);
+		/* NOTE: intentional use of uninitialized variable */
+		seed ^= (getpid() << 16) ^ tv.tv_sec ^ tv.tv_usec;
+	}
+	srandom(seed);
+}
+
 #ifdef TEST_PROGRAM
 int main(int argc __attribute__ ((__unused__)),
          char *argv[] __attribute__ ((__unused__)))
-- 
1.8.1

[PATCH 2/4] ipcmk: remove unnecessary create_key() function

[Sami Kerola]

Use seed_random() from randutils instead, and stop generating new seed
when it does not have to happen

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/ipcmk.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/sys-utils/ipcmk.c b/sys-utils/ipcmk.c
index a862ba1..9b943b7 100644
--- a/sys-utils/ipcmk.c
+++ b/sys-utils/ipcmk.c
@@ -34,30 +34,23 @@
 #include "nls.h"
 #include "strutils.h"
 #include "closestream.h"
-
-static key_t create_key(void)
-{
-	struct timeval now;
-	gettimeofday(&now, NULL);
-	srandom(now.tv_usec);
-	return random();
-}
+#include "randutils.h"
 
 static int create_shm(size_t size, int permission)
 {
-	key_t key = create_key();
+	key_t key = random();
 	return shmget(key, size, permission | IPC_CREAT);
 }
 
 static int create_msg(int permission)
 {
-	key_t key = create_key();
+	key_t key = random();
 	return msgget(key, permission | IPC_CREAT);
 }
 
 static int create_sem(int nsems, int permission)
 {
-	key_t key = create_key();
+	key_t key = random();
 	return semget(key, nsems, permission | IPC_CREAT);
 }
 
@@ -100,6 +93,7 @@ int main(int argc, char **argv)
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	atexit(close_stdout);
+	seed_random();
 
 	while((opt = getopt_long(argc, argv, "hM:QS:p:Vh", longopts, NULL)) != -1) {
 		switch(opt) {
-- 
1.8.1

[PATCH 3/4] ipcmk: allow user to create more ipc resources at a time

[Sami Kerola]

There is no reason why 'ipcmk -Q -Q' would should not work, so allow that
and any other combination of switches such as:

ipcmk -Q -Q -p 700 -M 42 -Q -S 13

The behavior of '-p' changed in this commit.  Permissions affect the
resources mentioned after the permission option.  Earlier the last
permission option took effect across different resources.  Notice that
formerly user could create one of each shm, msg, and sem simultaneously.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/ipcmk.1 |   3 +-
 sys-utils/ipcmk.c | 110 ++++++++++++++++++++++++++++--------------------------
 2 files changed, 59 insertions(+), 54 deletions(-)

diff --git a/sys-utils/ipcmk.1 b/sys-utils/ipcmk.1
index 0900a19..b9775a4 100644
--- a/sys-utils/ipcmk.1
+++ b/sys-utils/ipcmk.1
@@ -29,7 +29,8 @@ Message queue.
 .SH "ADDITIONAL OPTIONS"
 .TP
 \fB\-p\fR, \fB\-\-mode\fR [\fImode\fR]
-Permission for the resource. Default is 0644.
+Access permissions for the resource.  The mode will take effect for
+resources that are defined after the option.  Default mode is 0644.
 .TP
 \fB\-h\fR, \fB\-\-help\fR
 Display a short help message and exit.
diff --git a/sys-utils/ipcmk.c b/sys-utils/ipcmk.c
index 9b943b7..8d15028 100644
--- a/sys-utils/ipcmk.c
+++ b/sys-utils/ipcmk.c
@@ -36,22 +36,49 @@
 #include "closestream.h"
 #include "randutils.h"
 
-static int create_shm(size_t size, int permission)
+enum res_types {
+	SHM = 0,
+	MSG,
+	SEM
+};
+
+const char *res_messages[] = {
+	[SHM] = N_("shared memory"),
+	[MSG] = N_("message queue"),
+	[SEM] = N_("semaphore")
+};
+
+struct ipc_res {
+	int type;
+	int perm;
+	size_t size;
+	int nsems;
+	struct ipc_res *next;
+};
+
+static int create_res(struct ipc_res *res)
 {
+	int ret;
 	key_t key = random();
-	return shmget(key, size, permission | IPC_CREAT);
-}
-
-static int create_msg(int permission)
-{
-	key_t key = random();
-	return msgget(key, permission | IPC_CREAT);
-}
-
-static int create_sem(int nsems, int permission)
-{
-	key_t key = random();
-	return semget(key, nsems, permission | IPC_CREAT);
+	switch (res->type) {
+	case SHM:
+		ret = shmget(key, res->size, res->perm | IPC_CREAT);
+		break;
+	case MSG:
+		ret = msgget(key, res->perm | IPC_CREAT);
+		break;
+	case SEM:
+		ret = semget(key, res->nsems, res->perm | IPC_CREAT);
+		break;
+	default:
+		abort();
+	}
+	if (ret == -1) {
+		warn(_("create %s failed"), res_messages[res->type]);
+		return 1;
+	}
+	printf(_("%s id: %d\n"), res_messages[res->type], ret);
+	return 0;
 }
 
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
@@ -74,11 +101,9 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 
 int main(int argc, char **argv)
 {
-	int permission = 0644;
+	struct ipc_res res;
 	int opt;
-	size_t size = 0;
-	int nsems = 0;
-	int ask_shm = 0, ask_msg = 0, ask_sem = 0;
+	int ret = EXIT_SUCCESS;
 	static const struct option longopts[] = {
 		{"shmem", required_argument, NULL, 'M'},
 		{"semaphore", required_argument, NULL, 'S'},
@@ -93,23 +118,29 @@ int main(int argc, char **argv)
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	atexit(close_stdout);
+	if (argc < 2)
+		usage(stderr);
 	seed_random();
 
+	res.perm = 0644;
 	while((opt = getopt_long(argc, argv, "hM:QS:p:Vh", longopts, NULL)) != -1) {
 		switch(opt) {
 		case 'M':
-			size = strtou64_or_err(optarg, _("failed to parse size"));
-			ask_shm = 1;
+			res.type = SHM;
+			res.size = strtou64_or_err(optarg, _("failed to parse size"));
+			ret |= create_res(&res);
 			break;
 		case 'Q':
-			ask_msg = 1;
+			res.type = MSG;
+			ret |= create_res(&res);
 			break;
 		case 'S':
-			nsems = strtos32_or_err(optarg, _("failed to parse elements"));
-			ask_sem = 1;
+			res.type = SEM;
+			res.nsems = strtos32_or_err(optarg, _("failed to parse elements"));
+			ret |= create_res(&res);
 			break;
 		case 'p':
-			permission = strtoul(optarg, NULL, 8);
+			res.perm = strtoul(optarg, NULL, 8);
 			break;
 		case 'h':
 			usage(stdout);
@@ -118,37 +149,10 @@ int main(int argc, char **argv)
 			printf(UTIL_LINUX_VERSION);
 			return EXIT_SUCCESS;
 		default:
-			ask_shm = ask_msg = ask_sem = 0;
+			usage(stderr);
 			break;
 		}
 	}
 
-	if(!ask_shm && !ask_msg && !ask_sem)
-		usage(stderr);
-
-	if (ask_shm) {
-		int shmid;
-		if (-1 == (shmid = create_shm(size, permission)))
-			err(EXIT_FAILURE, _("create share memory failed"));
-		else
-			printf(_("Shared memory id: %d\n"), shmid);
-	}
-
-	if (ask_msg) {
-		int msgid;
-		if (-1 == (msgid = create_msg(permission)))
-			err(EXIT_FAILURE, _("create message queue failed"));
-		else
-			printf(_("Message queue id: %d\n"), msgid);
-	}
-
-	if (ask_sem) {
-		int semid;
-		if (-1 == (semid = create_sem(nsems, permission)))
-			err(EXIT_FAILURE, _("create semaphore failed"));
-		else
-			printf(_("Semaphore id: %d\n"), semid);
-	}
-
-	return EXIT_SUCCESS;
+	return ret;
 }
-- 
1.8.1

[PATCH 4/4] include: add missing values to sysfs_cxt initializer definition

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 include/sysfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sysfs.h b/include/sysfs.h
index 4f9c46b..739f9de 100644
--- a/include/sysfs.h
+++ b/include/sysfs.h
@@ -30,7 +30,7 @@ struct sysfs_cxt {
 	unsigned int	has_hctl : 1;
 };
 
-#define UL_SYSFSCXT_EMPTY { 0, -1, NULL, NULL }
+#define UL_SYSFSCXT_EMPTY { 0, -1, NULL, NULL, 0, 0, 0, 0, 0 }
 
 extern char *sysfs_devno_attribute_path(dev_t devno, char *buf,
                                  size_t bufsiz, const char *attr);
-- 
1.8.1

Re: [PATCH 3/4] ipcmk: allow user to create more ipc resources at a time

[Karel Zak]

On Wed, Jan 09, 2013 at 07:36:55PM +0000, Sami Kerola wrote:
> There is no reason why 'ipcmk -Q -Q' would should not work, so allow that
> and any other combination of switches such as:

 Well, we can implement whatever, but does it make sense? How many
 people look for the feature?

> ipcmk -Q -Q -p 700 -M 42 -Q -S 13
> 
> The behavior of '-p' changed in this commit. 

 Unacceptable, backwardly incompatible change.


 Sami, if you want to help then try to write tests, many many utils
 and features are still not covered by regression tests (hexdump,
 lsblk, ipc*, dmesg, blockdev, utmpdump, tailf, ....).

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/4] ipcmk: remove unnecessary create_key() function

[Karel Zak]

On Wed, Jan 09, 2013 at 07:36:54PM +0000, Sami Kerola wrote:
>  static int create_shm(size_t size, int permission)
>  {
> -	key_t key = create_key();
> +	key_t key = random();

 we already have random_get_bytes() in randutils.c, it seems better
 than introduce seed_random() and use random().

    random_get_bytes(&key, sizeof(key));


    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/4] ipcmk: remove unnecessary create_key() function

[Karel Zak]

On Thu, Jan 10, 2013 at 11:19:55AM +0100, Karel Zak wrote:
> On Wed, Jan 09, 2013 at 07:36:54PM +0000, Sami Kerola wrote:
> >  static int create_shm(size_t size, int permission)
> >  {
> > -	key_t key = create_key();
> > +	key_t key = random();
> 
>  we already have random_get_bytes() in randutils.c, it seems better
>  than introduce seed_random() and use random().
> 
>     random_get_bytes(&key, sizeof(key));

 Implemented.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 4/4] include: add missing values to sysfs_cxt initializer definition

[Karel Zak]

On Wed, Jan 09, 2013 at 07:36:56PM +0000, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  include/sysfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/sysfs.h b/include/sysfs.h
> index 4f9c46b..739f9de 100644
> --- a/include/sysfs.h
> +++ b/include/sysfs.h
> @@ -30,7 +30,7 @@ struct sysfs_cxt {
>  	unsigned int	has_hctl : 1;
>  };
>  
> -#define UL_SYSFSCXT_EMPTY { 0, -1, NULL, NULL }
> +#define UL_SYSFSCXT_EMPTY { 0, -1, NULL, NULL, 0, 0, 0, 0, 0 }

 Applied, but it's very probably unnecessary. All unspecified fields
 should be zeroized according to the standard.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com