jwrite_filep.patch Advance the file pointer after jwrite() and jwritev() mkdir_include.patch Include directory creation in make install open_const_char.patch The "pathname" argument to jopen() should be const lockf_locking.patch Remove some unneeded pthread_mutex_lock()s in the wrapper functions. strdup_checks.patch Typo in strdup() checks wrappers_jtrans_free.patch Fix a memory leak in the wrappers. jfsck_buf_free.patch Fix a very subtle bug when freeing a buffer in jfsck() diff -u cur-root/libjio.c cur-root/libjio.c --- cur-root/libjio.c 2004-04-05 12:32:08.000000000 -0300 +++ cur-root/libjio.c 2004-04-05 12:32:09.000000000 -0300 @@ -101,7 +101,7 @@ } /* build the journal directory name out of the filename */ -static int get_jdir(char *filename, char *jdir) +static int get_jdir(const char *filename, char *jdir) { char *base, *baset; char *dir, *dirt; @@ -112,7 +112,7 @@ base = basename(baset); dirt = strdup(filename); - if (baset == NULL) + if (dirt == NULL) return 0; dir = dirname(dirt); @@ -125,7 +125,7 @@ } /* build the filename of a given transaction */ -static int get_jtfile(char *filename, int tid, char *jtfile) +static int get_jtfile(const char *filename, int tid, char *jtfile) { char *base, *baset; char *dir, *dirt; @@ -136,7 +136,7 @@ base = basename(baset); dirt = strdup(filename); - if (baset == NULL) + if (dirt == NULL) return 0; dir = dirname(dirt); @@ -246,7 +246,7 @@ ts->plen = 0; } -/* free a transaction structure */ +/* free the contents of a transaction structure */ void jtrans_free(struct jtrans *ts) { /* NOTE: we only really free the name and previous data, which are the @@ -256,7 +256,10 @@ free(ts->name); if (ts->pdata) free(ts->pdata); - free(ts); + + /* don't free ts itself, it's very common to allocate it in the stack, + * so let the caller take care of it; and, after all, he was the one + * doing the alloc in the first place */ } /* commit a transaction */ @@ -440,7 +443,7 @@ */ /* open a file */ -int jopen(struct jfs *fs, char *name, int flags, int mode, int jflags) +int jopen(struct jfs *fs, const char *name, int flags, int mode, int jflags) { int fd, jfd, rv; unsigned int t; @@ -452,9 +455,22 @@ return -1; fs->fd = fd; - fs->name = name; + fs->name = strdup(name); fs->flags = jflags; + /* Note on fs->lock usage: this lock is used only inside the wrappers, + * and exclusively to protect the file pointer. This means that it + * must only be held while performing operations that depend or alter + * the file pointer (jread, jreadv, jwrite, jwritev), but the others + * (jpread, jpwrite) are left unprotected because they can be + * performed in paralell as long as they don't affect the same portion + * of the file (this is protected by lockf). The lock doesn't slow + * things down tho: any threaded app MUST implement this kind of + * locking anyways if it wants to prevent data corruption, we only + * make it easier for them by taking care of it here. If performance + * is essential, the jpread/jpwrite functions should be used, just as + * real life. */ + pthread_mutex_init( &(fs->lock), NULL); if (!get_jdir(name, jdir)) @@ -494,10 +510,14 @@ return fd; } + +/* read() family wrappers */ + /* read wrapper */ ssize_t jread(struct jfs *fs, void *buf, size_t count) { int rv; + pthread_mutex_lock(&(fs->lock)); lockf(fs->fd, F_LOCK, count); rv = read(fs->fd, buf, count); @@ -511,6 +531,7 @@ ssize_t jpread(struct jfs *fs, void *buf, size_t count, off_t offset) { int rv; + plockf(fs->fd, F_LOCK, offset, count); rv = pread(fs->fd, buf, count, offset); plockf(fs->fd, F_ULOCK, offset, count); @@ -554,19 +575,27 @@ ts.len = count; rv = jtrans_commit(&ts); + + if (rv >= 0) { + /* if success, advance the file pointer */ + lseek(fs->fd, count, SEEK_CUR); + } pthread_mutex_unlock(&(fs->lock)); + + jtrans_free(&ts); + return rv; } +/* write family wrappers */ + /* pwrite wrapper */ ssize_t jpwrite(struct jfs *fs, void *buf, size_t count, off_t offset) { int rv; struct jtrans ts; - pthread_mutex_lock(&(fs->lock)); - jtrans_init(fs, &ts); ts.offset = offset; @@ -574,8 +603,9 @@ ts.len = count; rv = jtrans_commit(&ts); - - pthread_mutex_unlock(&(fs->lock)); + + jtrans_free(&ts); + return rv; } @@ -616,8 +646,16 @@ ts.len = sum; rv = jtrans_commit(&ts); + + if (rv >= 0) { + /* if success, advance the file pointer */ + lseek(fs->fd, count, SEEK_CUR); + } pthread_mutex_unlock(&(fs->lock)); + + jtrans_free(&ts); + return rv; } @@ -642,6 +680,9 @@ return -1; if (close(fs->jfd)) return -1; + if (fs->name) + /* allocated by strdup() in jopen() */ + free(fs->name); return 0; } @@ -726,8 +767,6 @@ goto loop; } - curts->name = tname; - /* load from disk, header first */ buf = (char *) malloc(J_DISKTFIXSIZE); if (buf == NULL) { @@ -738,6 +777,7 @@ rv = read(tfd, buf, J_DISKTFIXSIZE); if (rv != J_DISKTFIXSIZE) { res->broken_head++; + free(buf); goto loop; } @@ -746,6 +786,8 @@ curts->ulen = (size_t) *(buf + 16); curts->offset = (off_t) *(buf + 20); + free(buf); + /* if we got here, the transaction was not applied, so we * check if the transaction file is complete (we only need to * apply it) or not (so we can't do anything but ignore it) */ @@ -784,7 +826,6 @@ offset = J_DISKTFIXSIZE; rv = spread(tfd, curts->udata, curts->ulen, offset); if (rv != curts->ulen) { - printf("ULEN\n"); res->load_error++; goto loop; } @@ -793,7 +834,6 @@ offset = J_DISKTFIXSIZE + curts->ulen; rv = spread(tfd, curts->pdata, curts->plen, offset); if (rv != curts->plen) { - printf("PLEN\n"); res->load_error++; goto loop; } @@ -815,21 +855,30 @@ res->reapplied++; /* free the data we just allocated */ - if (curts->len) + if (curts->len) { free(curts->buf); - if (curts->plen) + curts->buf = NULL; + } + if (curts->plen) { free(curts->pdata); - if (curts->ulen) + curts->pdata = NULL; + } + if (curts->ulen) { free(curts->udata); + curts->udata = NULL; + } + if (curts->name) { + free(curts->name); + curts->name = NULL; + } loop: if (tfd > 0) close(tfd); - - res->total++; - if (buf) - free(buf); + free(curts); + + res->total++; } return 0; unchanged: --- cur/Makefile~mkdir_include 2004-04-01 23:31:09.000000000 -0300 +++ cur-root/Makefile 2004-04-02 19:51:12.000000000 -0300 @@ -20,11 +20,14 @@ jiofsck: jiofsck.o static $(CC) jiofsck.o libjio.a -lpthread -o jiofsck install: all + install -g root -o root -d $(PREFIX)/lib install -g root -o root -m 0755 libjio.so $(PREFIX)/lib install -g root -o root -m 0644 libjio.a $(PREFIX)/lib + install -g root -o root -d $(PREFIX)/include install -g root -o root -m 0644 libjio.h $(PREFIX)/include + install -g root -o root -d $(PREFIX)/bin install -g root -o root -m 0775 jiofsck $(PREFIX)/bin - install -g root -o root -m 0644 -d $(PREFIX)/man/man3 + install -g root -o root -d $(PREFIX)/man/man3 install -g root -o root -m 0644 doc/libjio.3 $(PREFIX)/man/man3/ @echo @echo "Please run ldconfig to update your library cache" unchanged: --- cur/libjio.h~open_const_char 2004-04-05 12:31:46.000000000 -0300 +++ cur-root/libjio.h 2004-04-05 12:31:46.000000000 -0300 @@ -65,7 +65,7 @@ struct disk_trans { /* basic operations */ -int jopen(struct jfs *fs, char *name, int flags, int mode, int jflags); +int jopen(struct jfs *fs, const char *name, int flags, int mode, int jflags); ssize_t jread(struct jfs *fs, void *buf, size_t count); ssize_t jpread(struct jfs *fs, void *buf, size_t count, off_t offset); ssize_t jreadv(struct jfs *fs, struct iovec *vector, int count); unchanged: --- cur/doc/libjio.3~open_const_char 2004-04-05 12:31:46.000000000 -0300 +++ cur-root/doc/libjio.3 2004-04-05 12:31:46.000000000 -0300 @@ -6,7 +6,7 @@ libjio - A library for Journaled I/O .B #include -.BI "int jopen(struct jfs *" fs ", char *" name ", int " flags ", int " mode ", int " jflags " ); +.BI "int jopen(struct jfs *" fs ", const char *" name ", int " flags ", int " mode ", int " jflags " ); .BI "ssize_t jread(struct jfs *" fs ", void *" buf ", size_t " count " );