[gentoo-commits] proj/openrc:master commit in: src/rc/

[Christian Ruppert]

commit:     6be8a0679b8d64ea5b99ea98839eab2ce129988b
Author:     Christian Ruppert <idl0r <AT> gentoo <DOT> org>
AuthorDate: Sat Jan 28 15:40:49 2012 +0000
Commit:     Christian Ruppert <idl0r <AT> gentoo <DOT> org>
CommitDate: Sat Jan 28 15:43:54 2012 +0000
URL:        http://git.overlays.gentoo.org/gitweb/?p=proj/openrc.git;a=commit;h=6be8a067

Do not loop flock()

There's no need to loop until flock was successfully as flock() would simply
block till a previous lock has been released.
There's more to do to fix it properly, see my comments in the patch.

Reported-by: James Le Cuirot <chewi <AT> aura-online.co.uk>
X-Gentoo-Bug: 360013
X-Gentoo-Bug-URL: https://bugs.gentoo.org/360013

---
 src/rc/runscript.c |   28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/rc/runscript.c b/src/rc/runscript.c
index 8e0ced9..b0d8084 100644
--- a/src/rc/runscript.c
+++ b/src/rc/runscript.c
@@ -288,6 +288,13 @@ cleanup(void)
 #endif
 }
 
+/* Buffer and lock all output messages so that we get readable content */
+/* FIXME: Use a dynamic lock file that contains the tty/pts as well.
+ * For example openrc-pts8.lock or openrc-tty1.lock.
+ * Using a static lock file makes no sense, esp. in multi-user environments.
+ * Why don't we use (f)printf, as it is thread-safe through POSIX already?
+ * Bug: 360013
+ */
 static int
 write_prefix(const char *buffer, size_t bytes, bool *prefixed)
 {
@@ -297,14 +304,20 @@ write_prefix(const char *buffer, size_t bytes, bool *prefixed)
 	ssize_t ret = 0;
 	int fd = fileno(stdout), lock_fd = -1;
 
-	/* Spin until we lock the prefix */
-	for (;;) {
-		lock_fd = open(PREFIX_LOCK, O_WRONLY | O_CREAT, 0664);
-		if (lock_fd != -1)
-			if (flock(lock_fd, LOCK_EX) == 0)
-				break;
-		close(lock_fd);
+	/*
+	 * Lock the prefix.
+	 * open() may fail here when running as user, as RC_SVCDIR may not be writable.
+	 */
+	lock_fd = open(PREFIX_LOCK, O_WRONLY | O_CREAT, 0664);
+
+	if (lock_fd != -1) {
+		if (flock(lock_fd, LOCK_EX) != 0)
+			eerror("flock() failed: %s", strerror(errno));
 	}
+#ifdef RC_DEBUG
+	else
+		ewarn("Couldn't open the prefix lock, please make sure you have enough permissions");
+#endif
 
 	for (i = 0; i < bytes; i++) {
 		/* We don't prefix eend calls (cursor up) */
@@ -332,6 +345,7 @@ write_prefix(const char *buffer, size_t bytes, bool *prefixed)
 
 	/* Release the lock */
 	close(lock_fd);
+
 	return ret;
 }