From 27893e6651e64ad35f417ab665b8d1669fd03f61 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 5 Sep 2009 21:29:48 +0200 Subject: 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 --- libc/misc/utmp/utent.c | 81 +++++++++++++++++++++----------------------------- 1 file changed, 34 insertions(+), 47 deletions(-) (limited to 'libc/misc') diff --git a/libc/misc/utmp/utent.c b/libc/misc/utmp/utent.c index cc036cb6d..f0700af21 100644 --- a/libc/misc/utmp/utent.c +++ b/libc/misc/utmp/utent.c @@ -20,55 +20,43 @@ #include #include -/* 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 __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) { - if ((static_fd = open(static_ut_name, O_RDWR)) < 0) { - if ((static_fd = open(static_ut_name, O_RDONLY)) < 0) { - goto bummer; + if (static_fd < 0) { + static_fd = open(static_ut_name, O_RDWR); + if (static_fd < 0) { + static_fd = open(static_ut_name, O_RDONLY); + if (static_fd < 0) { + return; /* static_fd remains < 0 */ } } /* Make sure the file will be closed on exec() */ - ret = fcntl(static_fd, F_GETFD, 0); - if (ret >= 0) { - ret = fcntl(static_fd, F_SETFD, ret | FD_CLOEXEC); - } - if (ret < 0) { -bummer: - static_fd = -1; - close(static_fd); - return; - } + fcntl(static_fd, F_SETFD, FD_CLOEXEC); + // 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); @@ -78,19 +66,18 @@ 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; + } } - 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)) { ret = &static_utmp; } @@ -100,7 +87,7 @@ static struct utmp *__getutent(int utmp_fd) void endutent(void) { __UCLIBC_MUTEX_LOCK(utmplock); - if (static_fd != -1) + if (static_fd >= 0) close(static_fd); static_fd = -1; __UCLIBC_MUTEX_UNLOCK(utmplock); @@ -111,7 +98,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; } @@ -121,7 +108,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 || @@ -143,7 +130,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; @@ -160,7 +146,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; @@ -170,7 +156,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 @@ -188,7 +174,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) { @@ -202,9 +188,10 @@ int utmpname (const char *new_ut_name) } } - if (static_fd != -1) + if (static_fd >= 0) { close(static_fd); - static_fd = -1; + static_fd = -1; + } __UCLIBC_MUTEX_UNLOCK(utmplock); - return 0; + return 0; /* or maybe return -(static_ut_name != new_ut_name)? */ } -- cgit v1.2.3