From: Rusty Russell Date: Tue, 15 Feb 2011 12:53:59 +0000 (+1030) Subject: failtest: rely on the save/restore of files, don't use write cleanup. X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=5adceea6af51c93d3b6fc39849ff05340cd87253 failtest: rely on the save/restore of files, don't use write cleanup. --- diff --git a/ccan/failtest/failtest.c b/ccan/failtest/failtest.c index 2a39679f..56a1ad18 100644 --- a/ccan/failtest/failtest.c +++ b/ccan/failtest/failtest.c @@ -235,24 +235,23 @@ struct saved_file { struct saved_file *next; int fd; void *contents; - off_t len; + off_t off, len; }; static struct saved_file *save_file(struct saved_file *next, int fd) { struct saved_file *s = malloc(sizeof(*s)); - off_t orig = lseek(fd, 0, SEEK_CUR); - - /* Special file? Erk... */ - assert(orig != -1); s->next = next; s->fd = fd; + s->off = lseek(fd, 0, SEEK_CUR); + /* Special file? Erk... */ + assert(s->off != -1); s->len = lseek(fd, 0, SEEK_END); lseek(fd, 0, SEEK_SET); s->contents = malloc(s->len); read(fd, s->contents, s->len); - lseek(fd, orig, SEEK_SET); + lseek(fd, s->off, SEEK_SET); return s; } @@ -264,34 +263,26 @@ static struct saved_file *save_files(void) { struct saved_file *files = NULL; int i; - fd_set closed; /* Figure out the set of live fds. */ - FD_ZERO(&closed); for (i = history_num - 2; i >= 0; i--) { - /* FIXME: Handle dup. */ - if (history[i].type == FAILTEST_CLOSE) { - assert(!FD_ISSET(history[i].u.close.fd, &closed)); - FD_SET(history[i].u.close.fd, &closed); - } else if (history[i].type == FAILTEST_OPEN) { + if (history[i].type == FAILTEST_OPEN) { int fd = history[i].u.open.ret; /* Only do successful, writable fds. */ if (fd < 0) continue; - /* If it wasn't closed again... */ - if (!FD_ISSET(fd, &closed)) { - if ((history[i].u.open.flags & O_RDWR) - == O_RDWR) { - files = save_file(files, fd); - } else if ((history[i].u.open.flags & O_WRONLY) - == O_WRONLY) { - /* FIXME: Handle O_WRONLY. Open with - * O_RDWR? */ - abort(); - } - } else - FD_CLR(history[i].u.open.ret, &closed); + /* If it was closed, cleanup == NULL. */ + if (!history[i].cleanup) + continue; + + if ((history[i].u.open.flags & O_RDWR) == O_RDWR) { + files = save_file(files, fd); + } else if ((history[i].u.open.flags & O_WRONLY) + == O_WRONLY) { + /* FIXME: Handle O_WRONLY. Open with O_RDWR? */ + abort(); + } } } @@ -302,12 +293,12 @@ static void restore_files(struct saved_file *s) { while (s) { struct saved_file *next = s->next; - off_t orig = lseek(s->fd, 0, SEEK_CUR); lseek(s->fd, 0, SEEK_SET); write(s->fd, s->contents, s->len); + ftruncate(s->fd, s->len); free(s->contents); - lseek(s->fd, orig, SEEK_SET); + lseek(s->fd, s->off, SEEK_SET); free(s); s = next; } @@ -604,7 +595,6 @@ int failtest_open(const char *pathname, } else { p->u.open.ret = open(pathname, call.flags, call.mode); set_cleanup(p, cleanup_open, struct open_call); - p->u.open.dup_fd = p->u.open.ret; } errno = p->error; return p->u.open.ret; @@ -639,11 +629,6 @@ int failtest_pipe(int pipefd[2], const char *file, unsigned line) return p->u.pipe.ret; } -static void cleanup_read(struct read_call *call) -{ - lseek(call->fd, call->off, SEEK_SET); -} - ssize_t failtest_pread(int fd, void *buf, size_t count, off_t off, const char *file, unsigned line) { @@ -661,59 +646,23 @@ ssize_t failtest_pread(int fd, void *buf, size_t count, off_t off, p->error = EIO; } else { p->u.read.ret = pread(fd, buf, count, off); - set_cleanup(p, cleanup_read, struct read_call); } errno = p->error; return p->u.read.ret; } -static void cleanup_write(struct write_call *call) -{ - lseek(call->dup_fd, call->off, SEEK_SET); - write(call->dup_fd, call->saved_contents, call->saved_len); - lseek(call->dup_fd, call->off, SEEK_SET); - ftruncate(call->dup_fd, call->old_filelen); - free(call->saved_contents); -} - ssize_t failtest_pwrite(int fd, const void *buf, size_t count, off_t off, const char *file, unsigned line) { struct failtest_call *p; struct write_call call; - call.fd = call.dup_fd = fd; + call.fd = fd; call.buf = buf; call.count = count; call.off = off; p = add_history(FAILTEST_WRITE, file, line, &call); - /* Save old contents if we can */ - if (p->u.write.off != -1) { - ssize_t ret; - p->u.write.old_filelen = lseek(fd, 0, SEEK_END); - - /* Write past end of file? Nothing to save.*/ - if (p->u.write.old_filelen <= p->u.write.off) - p->u.write.saved_len = 0; - /* Write which goes over end of file? Partial save. */ - else if (p->u.write.off + count > p->u.write.old_filelen) - p->u.write.saved_len = p->u.write.old_filelen - - p->u.write.off; - /* Full save. */ - else - p->u.write.saved_len = count; - - p->u.write.saved_contents = malloc(p->u.write.saved_len); - lseek(fd, p->u.write.off, SEEK_SET); - ret = read(fd, p->u.write.saved_contents, p->u.write.saved_len); - if (ret != p->u.write.saved_len) - err(1, "Expected %i bytes, got %i", - (int)p->u.write.saved_len, (int)ret); - lseek(fd, p->u.write.off, SEEK_SET); - set_cleanup(p, cleanup_write, struct write_call); - } - /* If we're a child, tell parent about write. */ if (control_fd != -1) { enum info_type type = WRITE; @@ -841,49 +790,39 @@ add_lock(struct lock_info *locks, int fd, off_t start, off_t end, int type) return locks; } -/* We only trap this so we can dup fds in case we need to restore. */ +/* We trap this so we can record it: we don't fail it. */ int failtest_close(int fd) { - int new_fd = -1, i; + int i; if (fd < 0) return close(fd); - /* Trace history to find source of fd, and if we need to cleanup writes. */ + /* Trace history to find source of fd. */ for (i = history_num-1; i >= 0; i--) { switch (history[i].type) { - case FAILTEST_WRITE: - if (history[i].u.write.fd != fd) - break; - if (!history[i].cleanup) - break; - /* We need to save fd so we can restore file. */ - if (new_fd == -1) - new_fd = dup(fd); - history[i].u.write.dup_fd = new_fd; - break; - case FAILTEST_READ: - /* We don't need to cleanup reads on closed fds. */ - if (history[i].u.read.fd != fd) - break; - history[i].cleanup = NULL; - break; case FAILTEST_PIPE: - /* From a pipe? We don't ever restore pipes... */ + /* From a pipe? */ if (history[i].u.pipe.fds[0] == fd) { - assert(new_fd == -1); + assert(!history[i].u.pipe.closed[0]); history[i].u.pipe.closed[0] = true; + if (history[i].u.pipe.closed[1]) + history[i].cleanup = NULL; goto out; } if (history[i].u.pipe.fds[1] == fd) { - assert(new_fd == -1); + assert(!history[i].u.pipe.closed[1]); history[i].u.pipe.closed[1] = true; + if (history[i].u.pipe.closed[0]) + history[i].cleanup = NULL; goto out; } break; case FAILTEST_OPEN: if (history[i].u.open.ret == fd) { - history[i].u.open.dup_fd = new_fd; + assert((void *)history[i].cleanup + == (void *)cleanup_open); + history[i].cleanup = NULL; goto out; } break; diff --git a/ccan/failtest/failtest.h b/ccan/failtest/failtest.h index 21bc9f7c..473f9e79 100644 --- a/ccan/failtest/failtest.h +++ b/ccan/failtest/failtest.h @@ -64,7 +64,6 @@ struct open_call { const char *pathname; int flags; mode_t mode; - int dup_fd; }; struct pipe_call { @@ -87,10 +86,6 @@ struct write_call { const void *buf; size_t count; off_t off; - off_t old_filelen; - off_t saved_len; - void *saved_contents; - int dup_fd; }; struct fcntl_call { diff --git a/ccan/failtest/test/run-write.c b/ccan/failtest/test/run-write.c index 54255170..d3d7c604 100644 --- a/ccan/failtest/test/run-write.c +++ b/ccan/failtest/test/run-write.c @@ -15,7 +15,11 @@ int main(void) plan_tests(5); - fd = open("run-write-scratchpad", O_RDWR|O_CREAT, 0600); + fd = failtest_open("run-write-scratchpad", "run-write.c", 1, + O_RDWR|O_CREAT, 0600); + /* Child will fail, ignore. */ + if (fd < 0) + failtest_exit(0); write(fd, buf, strlen(buf)); ok1(lseek(fd, 0, SEEK_CUR) == strlen(buf));