utent.c: fix a few bugs, and shrink a bit

bug #1:
    static_fd = -1;
    close(static_fd);
DOH!

bug #2:
    if (utmp_fd == -1) {
        __setutent();
    }
    if (utmp_fd == -1) {
        return NULL;
    }
if utmp_fd == -1, we call _setutent().
if __setutent() opens a fd, utmp_fd (a parameter)
wouldn't change, the second check is bogus.
We need to use static_fd instead in second check.
Which makes clear that having utmp_fd parameter
is wrong. See the patch for a complete fix.

Shrink comes from simplifying fcntl(static_fd, F_SETFD, FD_CLOEXEC):

    text           data     bss     dec     hex filename
-    661              8     384    1053     41d libc/misc/utmp/utent.o
+    604              8     384     996     3e4 libc/misc/utmp/utent.o

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Signed-off-by: Austin Foxley <austinf@cetoncorp.com>

1 files changed, 40 insertions, 58 deletions

diff --git a/libc/misc/utmp/utent.c b/libc/misc/utmp/utent.c
index 11908b667..dc3110fe9 100644
--- a/libc/misc/utmp/utent.c
+++ b/libc/misc/utmp/utent.c
@@ -24,72 +24,55 @@
 #include <not-cancel.h>
 #endif
 
-/* Experimentally off - libc_hidden_proto(strcmp) */
-/* Experimentally off - libc_hidden_proto(strdup) */
-/* Experimentally off - libc_hidden_proto(strncmp) */
-/* libc_hidden_proto(read) */
-/* libc_hidden_proto(write) */
-/* libc_hidden_proto(open) */
-/* libc_hidden_proto(fcntl) */
-/* libc_hidden_proto(close) */
-/* libc_hidden_proto(lseek) */
-
 #include <bits/uClibc_mutex.h>
 __UCLIBC_MUTEX_STATIC(utmplock, PTHREAD_MUTEX_INITIALIZER);
 
 
-
 /* Some global crap */
 static int static_fd = -1;
 static struct utmp static_utmp;
 static const char default_file_name[] = _PATH_UTMP;
-static const char *static_ut_name = (const char *) default_file_name;
+static const char *static_ut_name = default_file_name;
 
 /* This function must be called with the LOCK held */
 static void __setutent(void)
 {
-    int ret;
-
-    if (static_fd == -1) {
-#ifdef __UCLIBC_HAS_THREADS_NATIVE__
-	if ((static_fd = open_not_cancel_2(static_ut_name, O_RDWR)) < 0) {
-	    if ((static_fd = open_not_cancel_2(static_ut_name, O_RDONLY)) < 0) {
-#else
-	if ((static_fd = open(static_ut_name, O_RDWR)) < 0) {
-	    if ((static_fd = open(static_ut_name, O_RDONLY)) < 0) {
-#endif
-		goto bummer;
-	    }
-	}
-	/* Make sure the file will be closed on exec()  */
+    if (static_fd < 0) {
 #ifdef __UCLIBC_HAS_THREADS_NATIVE__
-	ret = fcntl_not_cancel(static_fd, F_GETFD, 0);
+	static_fd = open(static_ut_name, O_RDWR);
 #else
-	ret = fcntl(static_fd, F_GETFD, 0);
+	static_fd = open_not_cancel_2(static_ut_name, O_RDWR);
 #endif
-	if (ret >= 0) {
+	if (static_fd < 0) {
 #ifdef __UCLIBC_HAS_THREADS_NATIVE__
-	    ret = fcntl_not_cancel(static_fd, F_SETFD, ret | FD_CLOEXEC);
+	    static_fd = open(static_ut_name, O_RDONLY);
 #else
-	    ret = fcntl(static_fd, F_SETFD, ret | FD_CLOEXEC);
+	    static_fd = open(static_ut_name, O_RDONLY);
 #endif
+	    if (static_fd < 0) {
+		return; /* static_fd remains < 0 */
+	    }
 	}
-	if (ret < 0) {
-bummer:
-	    static_fd = -1;
+	/* Make sure the file will be closed on exec()  */
 #ifdef __UCLIBC_HAS_THREADS_NATIVE__
-	    close_not_cancel_no_status(static_fd);
+	fcntl_not_cancel(static_fd, F_SETFD, FD_CLOEXEC);
 #else
-	    close(static_fd);
+	fcntl(static_fd, F_SETFD, FD_CLOEXEC);
 #endif
-	    return;
-	}
+	// thus far, {G,S}ETFD only has this single flag,
+	// and setting it never fails.
+	//int ret = fcntl(static_fd, F_GETFD, 0);
+	//if (ret >= 0) {
+	//    ret = fcntl(static_fd, F_SETFD, ret | FD_CLOEXEC);
+	//}
+	//if (ret < 0) {
+	//    static_fd = -1;
+	//}
+	return;
     }
     lseek(static_fd, 0, SEEK_SET);
-    return;
 }
 
-/* libc_hidden_proto(setutent) */
 void setutent(void)
 {
     __UCLIBC_MUTEX_LOCK(utmplock);
@@ -99,23 +82,22 @@ void setutent(void)
 libc_hidden_def(setutent)
 
 /* This function must be called with the LOCK held */
-static struct utmp *__getutent(int utmp_fd)
+static struct utmp *__getutent(void)
 {
     struct utmp *ret = NULL;
 
-    if (utmp_fd == -1) {
+    if (static_fd < 0) {
 	__setutent();
-    }
-    if (utmp_fd == -1) {
-	return NULL;
+	if (static_fd < 0) {
+	    return NULL;
+	}
     }
 
 #ifdef __UCLIBC_HAS_THREADS_NATIVE__
-    if (read_not_cancel(utmp_fd, (char *) &static_utmp, sizeof(struct utmp)) == sizeof(struct utmp)) 
+    if (read_not_cancel(static_fd, &static_utmp, sizeof(static_utmp)) == sizeof(static_utmp)) {
 #else
-    if (read(utmp_fd, (char *) &static_utmp, sizeof(struct utmp)) == sizeof(struct utmp))
+    if (read(static_fd, &static_utmp, sizeof(static_utmp)) == sizeof(static_utmp)) {
 #endif
-    {
 	ret = &static_utmp;
     }
 
@@ -125,7 +107,7 @@ static struct utmp *__getutent(int utmp_fd)
 void endutent(void)
 {
     __UCLIBC_MUTEX_LOCK(utmplock);
-    if (static_fd != -1)
+    if (static_fd >= 0)
 #ifdef __UCLIBC_HAS_THREADS_NATIVE__
 	close_not_cancel_no_status(static_fd);
 #else
@@ -140,7 +122,7 @@ struct utmp *getutent(void)
     struct utmp *ret = NULL;
 
     __UCLIBC_MUTEX_LOCK(utmplock);
-    ret = __getutent(static_fd);
+    ret = __getutent();
     __UCLIBC_MUTEX_UNLOCK(utmplock);
     return ret;
 }
@@ -150,7 +132,7 @@ static struct utmp *__getutid(const struct utmp *utmp_entry)
 {
     struct utmp *lutmp;
 
-    while ((lutmp = __getutent(static_fd)) != NULL) {
+    while ((lutmp = __getutent()) != NULL) {
 		if (	(utmp_entry->ut_type == RUN_LVL ||
 				 utmp_entry->ut_type == BOOT_TIME ||
 				 utmp_entry->ut_type == NEW_TIME ||
@@ -172,7 +154,6 @@ static struct utmp *__getutid(const struct utmp *utmp_entry)
     return NULL;
 }
 
-/* libc_hidden_proto(getutid) */
 struct utmp *getutid(const struct utmp *utmp_entry)
 {
     struct utmp *ret = NULL;
@@ -189,7 +170,7 @@ struct utmp *getutline(const struct utmp *utmp_entry)
     struct utmp *lutmp = NULL;
 
     __UCLIBC_MUTEX_LOCK(utmplock);
-    while ((lutmp = __getutent(static_fd)) != NULL) {
+    while ((lutmp = __getutent()) != NULL) {
 	if ((lutmp->ut_type == USER_PROCESS || lutmp->ut_type == LOGIN_PROCESS) &&
 		!strcmp(lutmp->ut_line, utmp_entry->ut_line)) {
 	    break;
@@ -199,7 +180,7 @@ struct utmp *getutline(const struct utmp *utmp_entry)
     return lutmp;
 }
 
-struct utmp *pututline (const struct utmp *utmp_entry)
+struct utmp *pututline(const struct utmp *utmp_entry)
 {
     __UCLIBC_MUTEX_LOCK(utmplock);
     /* Ignore the return value.  That way, if they've already positioned
@@ -217,7 +198,7 @@ struct utmp *pututline (const struct utmp *utmp_entry)
     return (struct utmp *)utmp_entry;
 }
 
-int utmpname (const char *new_ut_name)
+int utmpname(const char *new_ut_name)
 {
     __UCLIBC_MUTEX_LOCK(utmplock);
     if (new_ut_name != NULL) {
@@ -231,13 +212,14 @@ int utmpname (const char *new_ut_name)
 	}
     }
 
-    if (static_fd != -1)
+    if (static_fd >= 0) {
 #ifdef __UCLIBC_HAS_THREADS_NATIVE__
 	close_not_cancel_no_status(static_fd);
 #else
 	close(static_fd);
 #endif
-    static_fd = -1;
+	static_fd = -1;
+    }
     __UCLIBC_MUTEX_UNLOCK(utmplock);
-    return 0;
+    return 0; /* or maybe return -(static_ut_name != new_ut_name)? */
 }