aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff Dike <jdike@addtoit.com>2006-01-06 00:19:01 -0800
committerLinus Torvalds <torvalds@g5.osdl.org>2006-01-06 08:33:47 -0800
commit7eebe8a9c51686927709a57b1f2725d371014abc (patch)
tree6b1543982c8656c76c25baa7e44497d5d9de307a
parent2264c475e4bf7427e59921953c89a5693ecb506f (diff)
[PATCH] uml: umid cleanup
This patch cleans up the umid code: - The only_if_set argument to get_umid is gone. - get_umid returns an empty string rather than NULL if there is no umid. - umid_is_random is gone since its users went away. - Some printfs were turned into printks because the code runs late enough that printk is working. - Error paths were cleaned up. - Some functions now return an error and let the caller print the error message rather than printing it themselves. This eliminates the practice of passing a pointer to printf or printk in, depending on where in the boot process we are. - Major tidying of not_dead_yet - mostly error path cleanup, plus a comment explaining why it doesn't react to errors the way you might expect. - Calls to os_* interfaces that were moved under os are changed back to their native libc forms. - snprintf, strlcpy, and their bounds-checking friends are used more often, replacing by-hand bounds checking in some places. Signed-off-by: Jeff Dike <jdike@addtoit.com> Cc: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r--arch/um/drivers/line.c4
-rw-r--r--arch/um/include/os.h4
-rw-r--r--arch/um/include/user_util.h1
-rw-r--r--arch/um/kernel/um_arch.c4
-rw-r--r--arch/um/kernel/umid.c12
-rw-r--r--arch/um/os-Linux/umid.c265
6 files changed, 164 insertions, 126 deletions
diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
index b8e3e800ee4..a3c39373adb 100644
--- a/arch/um/drivers/line.c
+++ b/arch/um/drivers/line.c
@@ -831,8 +831,8 @@ char *add_xterm_umid(char *base)
char *umid, *title;
int len;
- umid = get_umid(1);
- if(umid == NULL)
+ umid = get_umid();
+ if(*umid == '\0')
return base;
len = strlen(base) + strlen(" ()") + strlen(umid) + 1;
diff --git a/arch/um/include/os.h b/arch/um/include/os.h
index 258444e5b9b..c279ee6d89e 100644
--- a/arch/um/include/os.h
+++ b/arch/um/include/os.h
@@ -216,7 +216,7 @@ extern int helper_wait(int pid);
/* umid.c */
extern int umid_file_name(char *name, char *buf, int len);
-extern int set_umid(char *name, int (*printer)(const char *fmt, ...));
-extern char *get_umid(int only_if_set);
+extern int set_umid(char *name);
+extern char *get_umid(void);
#endif
diff --git a/arch/um/include/user_util.h b/arch/um/include/user_util.h
index bb505e01d99..b9984003e60 100644
--- a/arch/um/include/user_util.h
+++ b/arch/um/include/user_util.h
@@ -64,7 +64,6 @@ extern void setup_machinename(char *machine_out);
extern void setup_hostinfo(void);
extern void do_exec(int old_pid, int new_pid);
extern void tracer_panic(char *msg, ...);
-extern char *get_umid(int only_if_set);
extern void do_longjmp(void *p, int val);
extern int detach(int pid, int sig);
extern int attach(int pid);
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 142a9493912..26626b2b917 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -146,8 +146,8 @@ void set_cmdline(char *cmd)
if(CHOOSE_MODE(honeypot, 0)) return;
- umid = get_umid(1);
- if(umid != NULL){
+ umid = get_umid();
+ if(*umid != '\0'){
snprintf(argv1_begin,
(argv1_end - argv1_begin) * sizeof(*ptr),
"(%s) ", umid);
diff --git a/arch/um/kernel/umid.c b/arch/um/kernel/umid.c
index 772c7cfbd8e..4eaee823bfd 100644
--- a/arch/um/kernel/umid.c
+++ b/arch/um/kernel/umid.c
@@ -3,15 +3,13 @@
* Licensed under the GPL
*/
-#include "linux/stddef.h"
-#include "linux/kernel.h"
#include "asm/errno.h"
#include "init.h"
#include "os.h"
#include "kern.h"
+#include "linux/kernel.h"
-/* Changed by set_umid_arg and umid_file_name */
-int umid_is_random = 0;
+/* Changed by set_umid_arg */
static int umid_inited = 0;
static int __init set_umid_arg(char *name, int *add)
@@ -22,11 +20,9 @@ static int __init set_umid_arg(char *name, int *add)
return 0;
*add = 0;
- err = set_umid(name, printf);
- if(err == -EEXIST){
+ err = set_umid(name);
+ if(err == -EEXIST)
printf("umid '%s' already in use\n", name);
- umid_is_random = 1;
- }
else if(!err)
umid_inited = 1;
diff --git a/arch/um/os-Linux/umid.c b/arch/um/os-Linux/umid.c
index 77d69a348cf..ecf107ae5ac 100644
--- a/arch/um/os-Linux/umid.c
+++ b/arch/um/os-Linux/umid.c
@@ -5,6 +5,7 @@
#include <errno.h>
#include <signal.h>
#include <dirent.h>
+#include <sys/fcntl.h>
#include <sys/stat.h>
#include <sys/param.h>
#include "init.h"
@@ -25,15 +26,16 @@ static char *uml_dir = UML_DIR;
static int __init make_uml_dir(void)
{
char dir[512] = { '\0' };
- int len;
+ int len, err;
if(*uml_dir == '~'){
char *home = getenv("HOME");
+ err = -ENOENT;
if(home == NULL){
- printf("make_uml_dir : no value in environment for "
+ printk("make_uml_dir : no value in environment for "
"$HOME\n");
- exit(1);
+ goto err;
}
strlcpy(dir, home, sizeof(dir));
uml_dir++;
@@ -43,18 +45,26 @@ static int __init make_uml_dir(void)
if (len > 0 && dir[len - 1] != '/')
strlcat(dir, "/", sizeof(dir));
+ err = -ENOMEM;
uml_dir = malloc(strlen(dir) + 1);
if (uml_dir == NULL) {
printf("make_uml_dir : malloc failed, errno = %d\n", errno);
- exit(1);
+ goto err;
}
strcpy(uml_dir, dir);
if((mkdir(uml_dir, 0777) < 0) && (errno != EEXIST)){
printf("Failed to mkdir '%s': %s\n", uml_dir, strerror(errno));
- return(-1);
+ err = -errno;
+ goto err_free;
}
return 0;
+
+err_free:
+ free(uml_dir);
+err:
+ uml_dir = NULL;
+ return err;
}
static int actually_do_remove(char *dir)
@@ -65,75 +75,88 @@ static int actually_do_remove(char *dir)
char file[256];
directory = opendir(dir);
- if(directory == NULL){
- printk("actually_do_remove : couldn't open directory '%s', "
- "errno = %d\n", dir, errno);
- return(1);
- }
+ if(directory == NULL)
+ return -errno;
+
while((ent = readdir(directory)) != NULL){
if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
continue;
len = strlen(dir) + sizeof("/") + strlen(ent->d_name) + 1;
- if(len > sizeof(file)){
- printk("Not deleting '%s' from '%s' - name too long\n",
- ent->d_name, dir);
- continue;
- }
+ if(len > sizeof(file))
+ return -E2BIG;
+
sprintf(file, "%s/%s", dir, ent->d_name);
- if(unlink(file) < 0){
- printk("actually_do_remove : couldn't remove '%s' "
- "from '%s', errno = %d\n", ent->d_name, dir,
- errno);
- return(1);
- }
- }
- if(rmdir(dir) < 0){
- printk("actually_do_remove : couldn't rmdir '%s', "
- "errno = %d\n", dir, errno);
- return(1);
+ if(unlink(file) < 0)
+ return -errno;
}
- return(0);
+ if(rmdir(dir) < 0)
+ return -errno;
+
+ return 0;
}
-extern int tracing_pid;
+/* This says that there isn't already a user of the specified directory even if
+ * there are errors during the checking. This is because if these errors
+ * happen, the directory is unusable by the pre-existing UML, so we might as
+ * well take it over. This could happen either by
+ * the existing UML somehow corrupting its umid directory
+ * something other than UML sticking stuff in the directory
+ * this boot racing with a shutdown of the other UML
+ * In any of these cases, the directory isn't useful for anything else.
+ */
static int not_dead_yet(char *dir)
{
char file[strlen(uml_dir) + UMID_LEN + sizeof("/pid\0")];
char pid[sizeof("nnnnn\0")], *end;
- int dead, fd, p, n;
+ int dead, fd, p, n, err;
+
+ n = snprintf(file, sizeof(file), "%s/pid", dir);
+ if(n >= sizeof(file)){
+ printk("not_dead_yet - pid filename too long\n");
+ err = -E2BIG;
+ goto out;
+ }
- sprintf(file, "%s/pid", dir);
dead = 0;
- fd = os_open_file(file, of_read(OPENFLAGS()), 0);
+ fd = open(file, O_RDONLY);
if(fd < 0){
if(fd != -ENOENT){
printk("not_dead_yet : couldn't open pid file '%s', "
"err = %d\n", file, -fd);
- return(1);
}
- dead = 1;
+ goto out;
}
- if(fd > 0){
- n = os_read_file(fd, pid, sizeof(pid));
- if(n < 0){
- printk("not_dead_yet : couldn't read pid file '%s', "
- "err = %d\n", file, -n);
- return(1);
- }
- p = strtoul(pid, &end, 0);
- if(end == pid){
- printk("not_dead_yet : couldn't parse pid file '%s', "
- "errno = %d\n", file, errno);
- dead = 1;
- }
- if(((kill(p, 0) < 0) && (errno == ESRCH)) ||
- (p == CHOOSE_MODE(tracing_pid, os_getpid())))
- dead = 1;
+
+ err = 0;
+ n = read(fd, pid, sizeof(pid));
+ if(n <= 0){
+ printk("not_dead_yet : couldn't read pid file '%s', "
+ "err = %d\n", file, -n);
+ goto out_close;
+ }
+
+ p = strtoul(pid, &end, 0);
+ if(end == pid){
+ printk("not_dead_yet : couldn't parse pid file '%s', "
+ "errno = %d\n", file, errno);
+ goto out_close;
}
- if(!dead)
- return(1);
- return(actually_do_remove(dir));
+
+ if((kill(p, 0) == 0) || (errno != ESRCH))
+ return 1;
+
+ err = actually_do_remove(dir);
+ if(err)
+ printk("not_dead_yet - actually_do_remove failed with "
+ "err = %d\n", err);
+
+ return err;
+
+ out_close:
+ close(fd);
+ out:
+ return 0;
}
static void __init create_pid_file(void)
@@ -145,26 +168,26 @@ static void __init create_pid_file(void)
if(umid_file_name("pid", file, sizeof(file)))
return;
- fd = os_open_file(file, of_create(of_excl(of_rdwr(OPENFLAGS()))),
- 0644);
+ fd = open(file, O_RDWR | O_CREAT | O_EXCL, 0644);
if(fd < 0){
- printf("Open of machine pid file \"%s\" failed: %s\n",
+ printk("Open of machine pid file \"%s\" failed: %s\n",
file, strerror(-fd));
return;
}
- sprintf(pid, "%d\n", os_getpid());
- n = os_write_file(fd, pid, strlen(pid));
+ snprintf(pid, sizeof(pid), "%d\n", getpid());
+ n = write(fd, pid, strlen(pid));
if(n != strlen(pid))
- printf("Write of pid file failed - err = %d\n", -n);
- os_close_file(fd);
+ printk("Write of pid file failed - err = %d\n", -n);
+
+ close(fd);
}
-int __init set_umid(char *name, int (*printer)(const char *fmt, ...))
+int __init set_umid(char *name)
{
if(strlen(name) > UMID_LEN - 1)
- (*printer)("Unique machine name is being truncated to %d "
- "characters\n", UMID_LEN);
+ return -E2BIG;
+
strlcpy(umid, name, sizeof(umid));
return 0;
@@ -172,44 +195,56 @@ int __init set_umid(char *name, int (*printer)(const char *fmt, ...))
static int umid_setup = 0;
-int __init make_umid(int (*printer)(const char *fmt, ...))
+int __init make_umid(void)
{
int fd, err;
char tmp[256];
+ if(umid_setup)
+ return 0;
+
make_uml_dir();
if(*umid == '\0'){
strlcpy(tmp, uml_dir, sizeof(tmp));
- strcat(tmp, "XXXXXX");
+ strlcat(tmp, "XXXXXX", sizeof(tmp));
fd = mkstemp(tmp);
if(fd < 0){
- (*printer)("make_umid - mkstemp(%s) failed: %s\n",
- tmp,strerror(errno));
- return(1);
+ printk("make_umid - mkstemp(%s) failed: %s\n",
+ tmp, strerror(errno));
+ err = -errno;
+ goto err;
}
- os_close_file(fd);
+ close(fd);
+
+ set_umid(&tmp[strlen(uml_dir)]);
+
/* There's a nice tiny little race between this unlink and
* the mkdir below. It'd be nice if there were a mkstemp
* for directories.
*/
- unlink(tmp);
- set_umid(&tmp[strlen(uml_dir)], printer);
+ if(unlink(tmp)){
+ err = -errno;
+ goto err;
+ }
}
- sprintf(tmp, "%s%s", uml_dir, umid);
+ snprintf(tmp, sizeof(tmp), "%s%s", uml_dir, umid);
err = mkdir(tmp, 0777);
if(err < 0){
- if(errno == EEXIST){
- if(not_dead_yet(tmp))
- return -EEXIST;
- err = mkdir(tmp, 0777);
- }
+ err = -errno;
+ if(errno != EEXIST)
+ goto err;
+
+ if(not_dead_yet(tmp) < 0)
+ goto err;
+
+ err = mkdir(tmp, 0777);
}
if(err < 0){
- (*printer)("Failed to create %s - errno = %d\n", umid, errno);
- return(-1);
+ printk("Failed to create '%s' - err = %d\n", umid, err);
+ goto err_rmdir;
}
umid_setup = 1;
@@ -217,13 +252,18 @@ int __init make_umid(int (*printer)(const char *fmt, ...))
create_pid_file();
return 0;
+
+ err_rmdir:
+ rmdir(tmp);
+ err:
+ return err;
}
static int __init make_umid_init(void)
{
- make_umid(printk);
+ make_umid();
- return(0);
+ return 0;
}
__initcall(make_umid_init);
@@ -232,48 +272,48 @@ int __init umid_file_name(char *name, char *buf, int len)
{
int n, err;
- if(!umid_setup){
- err = make_umid(printk);
- if(err)
- return err;
- }
+ err = make_umid();
+ if(err)
+ return err;
- n = strlen(uml_dir) + strlen(umid) + strlen("/") + strlen(name) + 1;
- if(n > len){
+ n = snprintf(buf, len, "%s%s/%s", uml_dir, umid, name);
+ if(n >= len){
printk("umid_file_name : buffer too short\n");
- return(-1);
+ return -E2BIG;
}
- sprintf(buf, "%s%s/%s", uml_dir, umid, name);
- return(0);
+ return 0;
}
-extern int umid_is_random;
-
-char *get_umid(int only_if_set)
+char *get_umid(void)
{
- if(only_if_set && umid_is_random)
- return NULL;
return umid;
}
static int __init set_uml_dir(char *name, int *add)
{
- if((strlen(name) > 0) && (name[strlen(name) - 1] != '/')){
- uml_dir = malloc(strlen(name) + 2);
- if(uml_dir == NULL){
- printf("Failed to malloc uml_dir - error = %d\n",
- errno);
- uml_dir = name;
- /* Return 0 here because do_initcalls doesn't look at
- * the return value.
- */
- return(0);
- }
- sprintf(uml_dir, "%s/", name);
+ if(*name == '\0'){
+ printf("uml_dir can't be an empty string\n");
+ return 0;
}
- else uml_dir = name;
- return(0);
+
+ if(name[strlen(name) - 1] == '/'){
+ uml_dir = name;
+ return 0;
+ }
+
+ uml_dir = malloc(strlen(name) + 2);
+ if(uml_dir == NULL){
+ printf("Failed to malloc uml_dir - error = %d\n", errno);
+
+ /* Return 0 here because do_initcalls doesn't look at
+ * the return value.
+ */
+ return 0;
+ }
+ sprintf(uml_dir, "%s/", name);
+
+ return 0;
}
__uml_setup("uml_dir=", set_uml_dir,
@@ -283,10 +323,13 @@ __uml_setup("uml_dir=", set_uml_dir,
static void remove_umid_dir(void)
{
- char dir[strlen(uml_dir) + UMID_LEN + 1];
+ char dir[strlen(uml_dir) + UMID_LEN + 1], err;
sprintf(dir, "%s%s", uml_dir, umid);
- actually_do_remove(dir);
+ err = actually_do_remove(dir);
+ if(err)
+ printf("remove_umid_dir - actually_do_remove failed with "
+ "err = %d\n", err);
}
__uml_exitcall(remove_umid_dir);