From 7ab2f3816c1ead09683415dc89898eb50a39f641 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 10 Feb 2016 17:09:11 +0000 Subject: [PATCH] transfused: address @yallop's comments --- alpine/packages/transfused/transfused.c | 336 ++++++++++-------------- 1 file changed, 136 insertions(+), 200 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index 097a88bcf..7e398d69d 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -40,14 +41,20 @@ typedef struct { char * fusermount = "fusermount"; +void die(int exit_code, const char * perror_arg, const char * fmt, ...) { + va_list argp; + va_start(argp, fmt); + vfprintf(stderr, fmt, argp); + va_end(argp); + if (perror_arg != NULL) perror(perror_arg); + exit(exit_code); +} + void * must_malloc(char *const descr, size_t size) { void * ptr; ptr = malloc(size); - if (size != 0 && ptr == NULL) { - perror(descr); - exit(1); - } + if (size != 0 && ptr == NULL) die(1,descr,""); return ptr; } @@ -60,30 +67,23 @@ char ** read_opts(connection_state * connection, char * buf) { char ** optv; if (asprintf(&read_path, "%s/connections/%ld/read", - connection->params->socket9p_root, connection->id) == -1) { - fprintf(stderr, "Couldn't allocate read path\n"); - exit(1); - } + connection->params->socket9p_root, connection->id) == -1) + die(1,"Couldn't allocate read path",""); read_fd = open(read_path, O_RDONLY); - if (read_fd == -1) { - fprintf(stderr, "For connection %ld, ", connection->id); - perror("couldn't open read path"); - exit(1); - } + if (read_fd == -1) + die(1,"couldn't open read path", "For connection %ld, ", connection->id); read_count = read(read_fd, buf, COPY_BUFSZ - 1); - if (read_count == -1) { - perror("read_opts error reading"); - exit(1); - } + if (read_count == -1) die(1,"read_opts error reading", ""); + buf[read_count] = 0x0; for (int i = 0; i < read_count; i++) { if (buf[i] == 0x0) optc++; } - optv = must_malloc("read_opts optv", (optc + 1) * sizeof(void *)); + optv = (char **) must_malloc("read_opts optv", (optc + 1) * sizeof(void *)); optv[0] = buf; optv[optc] = 0x0; @@ -100,28 +100,24 @@ char ** read_opts(connection_state * connection, char * buf) { return optv; } -uint64_t message_id(char * message) { - return *((uint64_t *) (message + 8)); +uint64_t message_id(uint64_t * message) { + return message[1]; } -int copy(copy_thread_state * copy_state) { +void copy(copy_thread_state * copy_state) { int from = copy_state->from; int to = copy_state->to; char * descr = copy_state->descr; int read_count, write_count; long connection = copy_state->connection; char * tag = copy_state->tag; - char * buf; + void * buf; buf = must_malloc(descr, COPY_BUFSZ); while(1) { read_count = read(from, buf, COPY_BUFSZ); - if (read_count == -1) { - fprintf(stderr, "copy %s: error reading ", descr); - perror(""); - exit(1); - } + if (read_count == -1) die(1, "", "copy %s: error reading: ", descr); if (save_trace) { int trace_fd; @@ -137,51 +133,42 @@ int copy(copy_thread_state * copy_state) { } write_count = write(to, buf, read_count); - if (write_count == -1) { - fprintf(stderr, "copy %s: error writing ", descr); - perror(""); - exit(1); - } + if (write_count == -1) die(1, "", "copy %s: error writing: ", descr); - if (write_count != read_count) { - fprintf(stderr, "copy %s: read %d but only wrote %d\n", - descr, read_count, write_count); - exit(1); - } + if (write_count != read_count) + die(1, NULL, "copy %s: read %d by only wrote %d\n", + descr, read_count, write_count); } free(buf); - return 0; } -int copy_clean_from(copy_thread_state * copy_state) { - int ret = copy(copy_state); +void * copy_clean_from(copy_thread_state * copy_state) { + copy(copy_state); - if (close(copy_state->from)) { - fprintf(stderr, "For %s, ", copy_state->descr); - perror("couldn't close read fd"); - exit(1); - } + if (close(copy_state->from)) + die(1, "couldn't close read fd", "For %s, ", copy_state->descr); free(copy_state->descr); free(copy_state); - return ret; + return NULL; } -int copy_clean_to(copy_thread_state * copy_state) { - int ret = copy(copy_state); +void * copy_clean_from_thread(void * copy_state) { + return (copy_clean_from((copy_thread_state *) copy_state)); +} - if (close(copy_state->to)) { - fprintf(stderr, "For %s, ", copy_state->descr); - perror("couldn't close write fd"); - exit(1); - } +void * copy_clean_to(copy_thread_state * copy_state) { + copy(copy_state); + + if (close(copy_state->to)) + die(1, "couldn't close write fd", "For %s, ", copy_state->descr); free(copy_state->descr); free(copy_state); - return ret; + return NULL; } int recv_fd(int sock) { @@ -207,17 +194,12 @@ int recv_fd(int sock) { ret = recvmsg(sock, &msg, 0); - if (ret == -1) { - perror("recvmsg"); - exit(1); - } + if (ret == -1) die(1,"recvmsg",""); if (ret > 0 && msg.msg_controllen > 0) { cmsg = CMSG_FIRSTHDR(&msg); if (cmsg->cmsg_level == SOL_SOCKET && (cmsg->cmsg_type == SCM_RIGHTS)) { fd = *(int*)CMSG_DATA(cmsg); - } else { - fprintf(stderr, "recvmsg: failed to receive an fd\n"); } } @@ -237,152 +219,116 @@ int get_fuse_sock(char *const optv[]) { // prepare argv from optv for (optc = 0; optv[optc] != NULL; optc++) {} - argv = must_malloc("fusermount argv",(optc + 2) * sizeof(char *)); + argv = (char **) must_malloc("fusermount argv",(optc + 2) * sizeof(char *)); argv[0] = fusermount; memcpy(&argv[1], optv, (optc + 1) * sizeof(char *)); // make the socket over which we'll be sent the FUSE socket fd - if (socketpair(PF_UNIX, SOCK_STREAM, 0, fuse_socks)) { - perror("Couldn't create FUSE socketpair"); - exit(1); - } + if (socketpair(PF_UNIX, SOCK_STREAM, 0, fuse_socks)) + die(1, "Couldn't create FUSE socketpair", ""); // prepare to exec the suid binary fusermount - if (asprintf(&envp[0], "_FUSE_COMMFD=%d", fuse_socks[0]) == -1) { - fprintf(stderr, "Couldn't allocate fusermount envp\n"); - exit(1); - } + if (asprintf(&envp[0], "_FUSE_COMMFD=%d", fuse_socks[0]) == -1) + die(1, "", "Couldn't allocate fusermount envp"); + envp[1] = 0x0; // fork and exec fusermount fusermount_pid = fork(); - if (!fusermount_pid) { // child - if (execvpe(fusermount, argv, envp)) { - perror("Failed to execute fusermount"); - exit(1); - } - } + if (!fusermount_pid) // child + if (execvpe(fusermount, argv, envp)) + die(1, "Failed to execute fusermount", ""); // parent free(argv); free(envp[0]); // close the end of the socket that we gave away - if (close(fuse_socks[0])) { - perror("Couldn't close unneeded fusermount socket"); - exit(1); - } - + if (close(fuse_socks[0])) + die(1, "Couldn't close unneeded fusermount socket", ""); + // wait for fusermount to return waitpid(fusermount_pid, &status, 0); - if (!WIFEXITED(status)) { - fprintf(stderr, "fusermount terminated abnormally\n"); - exit(1); - } - if (WEXITSTATUS(status)) { - fprintf(stderr, "fusermount exited with code %d\n", WEXITSTATUS(status)); - exit(1); - } + if (!WIFEXITED(status)) + die(1, NULL, "fusermount terminated abnormally\n"); + + if (WEXITSTATUS(status)) + die(1, NULL, "fusermount exited with code %d\n", WEXITSTATUS(status)); if (debug) fprintf(stderr, "about to recv_fd from fusermount\n"); fd = recv_fd(fuse_socks[1]); - if (fd == -1) { - fprintf(stderr, "Couldn't receive fd over FUSE socket\n"); - exit(1); - } + if (fd == -1) + die(1, NULL, "Couldn't receive fd over FUSE socket\n"); // close the read end of the socket - if (close(fuse_socks[1])) { - perror("Couldn't close fusermount read socket"); - exit(1); - } + if (close(fuse_socks[1])) + die(1, "Couldn't close fusermount read socket", ""); return fd; } -int start_reader(connection_state * connection, int fuse) { +void start_reader(connection_state * connection, int fuse) { int read_fd; char * read_path; pthread_t child; copy_thread_state * copy_state; - void *(*copy_clean)(void *) = (void *(*)(void *)) copy_clean_from; if (asprintf(&read_path, "%s/connections/%ld/read", - connection->params->socket9p_root, connection->id) == -1) { - fprintf(stderr, "Couldn't allocate read path\n"); - exit(1); - } + connection->params->socket9p_root, connection->id) == -1) + die(1, "", "Couldn't allocate read path: "); read_fd = open(read_path, O_RDONLY); - if (read_fd == -1) { - fprintf(stderr, "For connection %ld, ", connection->id); - perror("couldn't open read path"); - exit(1); - } + if (read_fd == -1) + die(1, "couldn't open read path", "For connection %ld, ", connection->id); - copy_state = must_malloc("start_reader copy_state", - sizeof(copy_thread_state)); + copy_state = (copy_thread_state *) must_malloc("start_reader copy_state", + sizeof(copy_thread_state)); copy_state->descr = read_path; copy_state->connection = connection->id; copy_state->tag = "read"; copy_state->from = read_fd; copy_state->to = fuse; - if ((errno = pthread_create(&child, NULL, copy_clean, copy_state))) { - fprintf(stderr, "couldn't create read copy thread for connection %ld ", - connection->id); - perror(""); - exit(1); - } + if ((errno = pthread_create(&child, NULL, + copy_clean_from_thread, copy_state))) + die(1, "", "couldn't create read copy thread for connection %ld: ", + connection->id); - if ((errno = pthread_detach(child))) { - fprintf(stderr, "couldn't detach read copy thread for connection '%ld' ", - connection->id); - perror(""); - exit(1); - } - - return 0; + if ((errno = pthread_detach(child))) + die (1, "", "couldn't detach read copy thread for connection '%ld': ", + connection->id); } -int start_writer(connection_state * connection, int fuse) { +void do_write(connection_state * connection, int fuse) { int write_fd; char * write_path; copy_thread_state * copy_state; if (asprintf(&write_path, "%s/connections/%ld/write", - connection->params->socket9p_root, connection->id) == -1) { - fprintf(stderr, "Couldn't allocate write path\n"); - exit(1); - } + connection->params->socket9p_root, connection->id) == -1) + die(1, "", "Couldn't allocate write path: "); write_fd = open(write_path, O_WRONLY); - if (write_fd == -1) { - fprintf(stderr, "For connection %ld, ", connection->id); - perror("couldn't open write path"); - exit(1); - } + if (write_fd == -1) + die(1, "couldn't open write path", "For connection %ld, ", connection->id); - copy_state = must_malloc("start_writer copy_state", - sizeof(copy_thread_state)); + copy_state = (copy_thread_state *) must_malloc("do_write copy_state", + sizeof(copy_thread_state)); copy_state->descr = write_path; copy_state->connection = connection->id; copy_state->tag = "write"; copy_state->from = fuse; copy_state->to = write_fd; copy_clean_to(copy_state); - - return 0; } -int handle_connection(connection_state * connection) { +void * handle_connection(connection_state * connection) { char ** optv; int fuse; char * buf; - int ret; - buf = must_malloc("read_opts packet malloc", COPY_BUFSZ); + buf = (char *) must_malloc("read_opts packet malloc", COPY_BUFSZ); optv = read_opts(connection, buf); fuse = get_fuse_sock(optv); @@ -390,10 +336,14 @@ int handle_connection(connection_state * connection) { free(buf); start_reader(connection, fuse); - ret = start_writer(connection, fuse); + do_write(connection, fuse); free(connection); - return ret; + return NULL; +} + +void * handle_connection_thread(void * connection) { + return handle_connection((connection_state *) connection); } void toggle_save_trace(int sig) { @@ -403,15 +353,11 @@ void toggle_save_trace(int sig) { void setup_save_trace() { save_trace = 0; - if (SIG_ERR == signal(SIGHUP, toggle_save_trace)) { - perror("Couldn't set SIGHUP behavior"); - exit(1); - } + if (SIG_ERR == signal(SIGHUP, toggle_save_trace)) + die(1, "Couldn't set SIGHUP behavior", ""); - if (siginterrupt(SIGHUP, 1)) { - perror("Couldn't set siginterrupt for SIGHUP"); - exit(1); - } + if (siginterrupt(SIGHUP, 1)) + die(1, "Couldn't set siginterrupt for SIGHUP", ""); } #define ID_LEN 512 @@ -419,11 +365,10 @@ void setup_save_trace() { int main(int argc, char * argv[]) { int events, read_count; char buf[ID_LEN]; - long conn; + long conn_id; pthread_t child; - void *(*handle)(void *) = (void *(*)(void *)) handle_connection; parameters params; - connection_state * connection; + connection_state * conn; char * events_path; if (argc < 2) { @@ -432,60 +377,51 @@ int main(int argc, char * argv[]) { params.socket9p_root = argv[1]; } - if (asprintf(&events_path, "%s/events", params.socket9p_root) == -1) { - fprintf(stderr, "Couldn't allocate events path\n"); - exit(1); - } + if (asprintf(&events_path, "%s/events", params.socket9p_root) == -1) + die(1, "", "Couldn't allocate events path: "); setup_save_trace(); events = open(events_path, O_RDONLY | O_CLOEXEC); - while (events != -1) { - read_count = read(events, buf, ID_LEN - 1); - if (read_count == -1) { - perror("Error reading events path"); - exit(1); - } else if (read_count == 0) { - // TODO: this is probably the 9p server's fault due to - // not dropping the read 0 to force short read if - // the real read is flushed - fprintf(stderr, "read 0 from event stream\n"); - continue; + if (events != -1) { + while (1) { + read_count = read(events, buf, ID_LEN - 1); + if (read_count == -1) { + die(1, "Error reading events path", ""); + } else if (read_count == 0) { + // TODO: this is probably the 9p server's fault due to + // not dropping the read 0 to force short read if + // the real read is flushed + fprintf(stderr, "read 0 from event stream\n"); + continue; + } + + buf[read_count] = 0x0; + + errno = 0; + conn_id = strtol(buf, NULL, 10); + if (errno) die(1, "failed", "Connection id of string '%s'", buf); + + if (debug) fprintf(stderr, "handle connection %ld\n", conn_id); + + conn = (connection_state *) must_malloc("connection state", + sizeof(connection_state)); + conn->id = conn_id; + conn->params = ¶ms; + + if ((errno = pthread_create(&child, NULL, + handle_connection_thread, conn))) + die(1, "", "Couldn't create thread for connection '%ld': ", conn_id); + + if ((errno = pthread_detach(child))) + die(1, "", "Couldn't detach thread for connection '%ld': ", conn_id); + + if (debug) fprintf(stderr, "thread spawned\n"); } - - buf[read_count] = 0x0; - - errno = 0; - conn = strtol(buf, NULL, 10); - if (errno) { - fprintf(stderr, "connection id of string '%s' ", buf); - perror("failed"); - exit(1); - } - - if (debug) fprintf(stderr, "handle connection %ld\n", conn); - - connection = must_malloc("connection state", sizeof(connection_state)); - connection->id = conn; - connection->params = ¶ms; - - if ((errno = pthread_create(&child, NULL, handle, connection))) { - fprintf(stderr, "couldn't create thread for connection '%ld' ", conn); - perror(""); - exit(1); - } - - if ((errno = pthread_detach(child))) { - fprintf(stderr, "couldn't detach thread for connection '%ld' ", conn); - perror(""); - exit(1); - } - - if (debug) fprintf(stderr, "thread spawned\n"); } - fprintf(stderr, "failed to open events path: %s\n", events_path); - + fprintf(stderr, "Failed to open events path %s: ", events_path); + perror(""); free(events_path); return 1; }