dbfs: add comments, and fix minor issues found while writing comments

- s/dbfs_unlink_info/dbfs_dirscan_info/
- only call putenv(3) to clear DB_PASSWORD after calling
  DB_ENV->set_encrypt()
- delete dbfs_dir_cmp(), dbfs_dir_scan1() is better
- update dbfs_lookup() to use dbfs_dir_scan1()
- guard min() macro inside #ifndef
- mark divisor 512 with ULL suffix
- mark on-disk structures with __attribute__ ((packed))

3 files changed, 87 insertions, 43 deletions

diff --git a/dbfs-backend.c b/dbfs-backend.c
index 736cbeb..3d0f579 100644
--- a/dbfs-backend.c
+++ b/dbfs-backend.c
@@ -19,7 +19,7 @@ struct dbfs_lookup_info {
 	guint64		*ino;
 };
 
-struct dbfs_unlink_info {
+struct dbfs_dirscan_info {
 	const char		*name;
 	size_t			namelen;
 	void			*start_ent;
@@ -45,11 +45,8 @@ void dbfs_init(void *userdata)
 		exit(1);
 	}
 
+	/* this isn't a very secure way to handle passwords */
 	db_password = getenv("DB_PASSWORD");
-	if (db_password) {
-		if (putenv("DB_PASSWORD=X"))
-			perror("putenv (SECURITY WARNING)");
-	}
 
 	rc = db_env_create(&db_env, 0);
 	if (rc) {
@@ -57,6 +54,7 @@ void dbfs_init(void *userdata)
 		exit(1);
 	}
 
+	/* stderr is wrong; should use syslog instead */
 	db_env->set_errfile(db_env, stderr);
 	db_env->set_errpfx(db_env, "dbfs");
 
@@ -67,8 +65,13 @@ void dbfs_init(void *userdata)
 			db_env->err(db_env, rc, "db_env->set_encrypt");
 			goto err_out;
 		}
+
+		/* this isn't a very good way to shroud the password */
+		if (putenv("DB_PASSWORD=X"))
+			perror("putenv (SECURITY WARNING)");
 	}
 
+	/* init DB transactional environment, stored in directory db_home */
 	rc = db_env->open(db_env, db_home,
 			  DB_INIT_LOCK | DB_INIT_LOG | DB_INIT_MPOOL |
 			  DB_INIT_TXN | DB_RECOVER | DB_CREATE | flags, 0666);
@@ -94,12 +97,17 @@ void dbfs_init(void *userdata)
 		goto err_out_meta;
 	}
 
+	/* our data items are small, so use the smallest possible page
+	 * size.  This is a guess, and should be verified by looking at
+	 * overflow pages and other DB statistics.
+	 */
 	rc = db_meta->set_pagesize(db_meta, 512);
 	if (rc) {
 		db_meta->err(db_meta, rc, "db_meta->set_pagesize");
 		goto err_out_meta;
 	}
 
+	/* fix everything as little endian */
 	rc = db_meta->set_lorder(db_meta, 1234);
 	if (rc) {
 		db_meta->err(db_meta, rc, "db_meta->set_lorder");
@@ -138,6 +146,7 @@ static int dbmeta_del(const char *key_str)
 	key.data = (void *) key_str;
 	key.size = strlen(key_str);
 
+	/* delete key 'key_str' from metadata database */
 	rc = db_meta->del(db_meta, NULL, &key, 0);
 	if (rc == DB_NOTFOUND)
 		return -ENOENT;
@@ -234,13 +243,16 @@ int dbfs_inode_read(guint64 ino_n, struct dbfs_inode **ino_out)
 	if (rc)
 		return rc;
 
+	/* calculate size of struct dbfs_extent area */
 	ex_sz = val.size - sizeof(struct dbfs_raw_inode);
 
+	/* initialize runtime information about the inode */
 	ino = g_new(struct dbfs_inode, 1);
 	ino->n_extents = ex_sz / sizeof(struct dbfs_extent);
 	ino->raw_ino_size = val.size;
 	ino->raw_inode = val.data;
 
+	/* deduce inode type */
 	mode = GUINT32_FROM_LE(ino->raw_inode->mode);
 	if (S_ISDIR(mode))
 		ino->type = IT_DIR;
@@ -338,10 +350,14 @@ int dbfs_dir_foreach(void *dir, dbfs_dir_actor_t func, void *userdata)
 		if (!de->namelen)
 			break;
 
+		/* send dirent to callback function */
 		rc = func(de, userdata);
 		if (rc)
 			break;
 
+		/* align things so that compiler structures
+		 * do not wind up misaligned.
+		 */
 		p += sizeof(struct dbfs_dirent) + de->namelen +
 		     (4 - (de->namelen & 0x3));
 	}
@@ -349,13 +365,17 @@ int dbfs_dir_foreach(void *dir, dbfs_dir_actor_t func, void *userdata)
 	return rc;
 }
 
-static int dbfs_dir_cmp(struct dbfs_dirent *de, void *userdata)
+static int dbfs_dir_scan1(struct dbfs_dirent *de, void *userdata)
 {
-	struct dbfs_lookup_info *li = userdata;
+	struct dbfs_dirscan_info *di = userdata;
 
-	if ((li->namelen == de->namelen) &&
-	    (!memcmp(li->name, de->name, li->namelen))) {
-	    	*li->ino = de->ino;
+	if (!di->start_ent) {
+		if ((de->namelen == di->namelen) &&
+		    (!memcmp(de->name, di->name, di->namelen)))
+			di->start_ent = de;
+	}
+	else if (!di->end_ent) {
+		di->end_ent = de;
 		return 1;
 	}
 
@@ -364,50 +384,43 @@ static int dbfs_dir_cmp(struct dbfs_dirent *de, void *userdata)
 
 int dbfs_lookup(guint64 parent, const char *name, guint64 *ino)
 {
-	struct dbfs_lookup_info li;
-	size_t namelen = strlen(name);
+	struct dbfs_dirscan_info di;
+	struct dbfs_dirent *de;
 	DBT val;
 	int rc;
 
 	*ino = 0;
 
+	/* read directory from database */
 	rc = dbfs_read_dir(parent, &val);
 	if (rc)
 		return rc;
 
-	li.name = name;
-	li.namelen = namelen;
-	li.ino = ino;
-	rc = dbfs_dir_foreach(val.data, dbfs_dir_cmp, &li);
-	if (rc)
-		rc = 0;
-	else
+	memset(&di, 0, sizeof(di));
+	di.name = name;
+	di.namelen = strlen(name);
+
+	/* query pointer to start of matching dirent */
+	rc = dbfs_dir_foreach(val.data, dbfs_dir_scan1, &di);
+	if (!rc || !di.start_ent) {
 		rc = -ENOENT;
+		goto out;
+	}
+	if (rc != 1)
+		goto out;
 
+	/* if match found, return inode number */
+	de = di.start_ent;
+	*ino = de->ino;
+
+out:
 	free(val.data);
 	return rc;
 }
 
-static int dbfs_dir_scan1(struct dbfs_dirent *de, void *userdata)
-{
-	struct dbfs_unlink_info *ui = userdata;
-
-	if (!ui->start_ent) {
-		if ((de->namelen == ui->namelen) &&
-		    (!memcmp(de->name, ui->name, ui->namelen)))
-			ui->start_ent = de;
-	}
-	else if (!ui->end_ent) {
-		ui->end_ent = de;
-		return 1;
-	}
-
-	return 0;
-}
-
 static int dbfs_dirent_del(guint64 parent, const char *name)
 {
-	struct dbfs_unlink_info ui;
+	struct dbfs_dirscan_info ui;
 	DBT dir_val;
 	int rc, del_len, tail_len;
 
@@ -419,6 +432,7 @@ static int dbfs_dirent_del(guint64 parent, const char *name)
 	ui.name = name;
 	ui.namelen = strlen(name);
 
+	/* query pointer to start of matching dirent */
 	rc = dbfs_dir_foreach(dir_val.data, dbfs_dir_scan1, &ui);
 	if (rc != 1) {
 		free(dir_val.data);
diff --git a/dbfs.c b/dbfs.c
index 111ddb3..79ead50 100644
--- a/dbfs.c
+++ b/dbfs.c
@@ -20,12 +20,16 @@ static void dbfs_op_getattr(fuse_req_t req, fuse_ino_t ino_n,
 	struct stat st;
 	int rc;
 
+	/* read inode from database */
 	rc = dbfs_inode_read(ino_n, &ino);
 	if (rc) {
 		fuse_reply_err(req, ENOENT);
 		return;
 	}
 
+	/* fill in stat buf, taking care to convert from
+	 * little endian to native endian
+	 */
 	memset(&st, 0, sizeof(st));
 	st.st_dev	= 1;
 	st.st_ino	= ino_n;
@@ -36,11 +40,12 @@ static void dbfs_op_getattr(fuse_req_t req, fuse_ino_t ino_n,
 	st.st_rdev	= GUINT64_FROM_LE(ino->raw_inode->rdev);
 	st.st_size	= GUINT64_FROM_LE(ino->raw_inode->size);
 	st.st_blksize	= 512;
-	st.st_blocks	= GUINT64_FROM_LE(ino->raw_inode->size) / 512;
+	st.st_blocks	= GUINT64_FROM_LE(ino->raw_inode->size) / 512ULL;
 	st.st_atime	= GUINT64_FROM_LE(ino->raw_inode->atime);
 	st.st_mtime	= GUINT64_FROM_LE(ino->raw_inode->mtime);
 	st.st_ctime	= GUINT64_FROM_LE(ino->raw_inode->ctime);
 
+	/* send result back to FUSE */
 	fuse_reply_attr(req, &st, 2.0);
 
 	dbfs_inode_free(ino);
@@ -52,12 +57,14 @@ static void dbfs_op_readlink(fuse_req_t req, fuse_ino_t ino)
 	DBT val;
 	char *s;
 
+	/* read link from database */
 	rc = dbfs_read_link(ino, &val);
 	if (rc) {
 		fuse_reply_err(req, rc);
 		return;
 	}
 
+	/* send reply; use g_strndup to append a trailing null */
 	s = g_strndup(val.data, val.size);
 	fuse_reply_readlink(req, s);
 	g_free(s);
@@ -71,14 +78,16 @@ static void dbfs_op_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
 	guint64 ino;
 	int rc;
 
+	/* lookup inode in parent directory */
 	rc = dbfs_lookup(parent, name, &ino);
 	if (rc) {
 		fuse_reply_err(req, rc);
 		return;
 	}
 
-	memset(&e, 0, sizeof(e));
+	/* send reply; timeout of 2.0 is just a guess */
 
+	memset(&e, 0, sizeof(e));
 	e.ino = ino;
 	e.attr_timeout = 2.0;
 	e.entry_timeout = 2.0;
@@ -92,14 +101,17 @@ static void dbfs_op_opendir(fuse_req_t req, fuse_ino_t ino,
 	DBT val;
 	int rc;
 
+	/* read directory from database */
 	rc = dbfs_read_dir(ino, &val);
 	if (rc) {
 		fuse_reply_err(req, rc);
 		return;
 	}
 
+	/* save for later use */
 	fi->fh = (uint64_t) (unsigned long) val.data;
 
+	/* send reply */
 	fuse_reply_open(req, fi);
 }
 
@@ -107,6 +119,8 @@ static void dbfs_op_releasedir(fuse_req_t req, fuse_ino_t ino,
 			       struct fuse_file_info *fi)
 {
 	void *p = (void *) (unsigned long) fi->fh;
+
+	/* release directory contents */
 	free(p);
 }
 
@@ -115,6 +129,7 @@ struct dirbuf {
 	size_t size;
 };
 
+/* stock function copied from FUSE template */
 static void dirbuf_add(struct dirbuf *b, const char *name, fuse_ino_t ino)
 {
 	struct stat stbuf;
@@ -126,8 +141,11 @@ static void dirbuf_add(struct dirbuf *b, const char *name, fuse_ino_t ino)
 	fuse_add_dirent(b->p + oldsize, name, &stbuf, b->size);
 }
 
+#ifndef min
 #define min(x, y) ((x) < (y) ? (x) : (y))
+#endif
 
+/* stock function copied from FUSE template */
 static int reply_buf_limited(fuse_req_t req, const char *buf, size_t bufsize,
 			     off_t off, size_t maxsize)
 {
@@ -141,7 +159,10 @@ static int reply_buf_limited(fuse_req_t req, const char *buf, size_t bufsize,
 static int dbfs_fill_dirbuf(struct dbfs_dirent *de, void *userdata)
 {
 	struct dirbuf *b = userdata;
-	char *s = g_strndup(de->name, de->namelen);
+	char *s;
+
+	/* add dirent to buffer; use g_strndup solely to append nul */
+	s = g_strndup(de->name, de->namelen);
 	dirbuf_add(b, s, de->ino);
 	free(s);
 	return 0;
@@ -153,11 +174,14 @@ static void dbfs_op_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
 	struct dirbuf b;
 	void *p;
 
+	/* grab directory contents stored by opendir */
 	p = (void *) (unsigned long) fi->fh;
 
+	/* iterate through each dirent, filling dirbuf */
 	memset(&b, 0, sizeof(b));
 	dbfs_dir_foreach(p, dbfs_fill_dirbuf, &b);
 
+	/* send reply */
 	reply_buf_limited(req, b.p, b.size, off, size);
 	free(b.p);
 }
@@ -184,20 +208,25 @@ static void dbfs_op_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name)
 	int rc;
 	DBT val;
 
+	/* get inode number associated with name */
 	rc = dbfs_lookup(parent, name, &ino_n);
 	if (rc)
 		goto err_out;
 
+	/* read dir associated with name */
 	rc = dbfs_read_dir(ino_n, &val);
 	if (rc)
 		goto err_out;
 
+	/* make sure dir only contains "." and ".." */
 	rc = dbfs_dir_foreach(val.data, dbfs_chk_empty, NULL);
 	free(val.data);
 
+	/* if dbfs_chk_empty() returns non-zero, dir is not empty */
 	if (rc)
 		goto err_out;
 
+	/* dir is empty, go ahead and unlink */
 	rc = dbfs_unlink(parent, name, DBFS_UNLINK_DIR);
 	if (rc)
 		goto err_out;
@@ -266,6 +295,7 @@ static struct fuse_lowlevel_ops dbfs_ops = {
 	.create		= NULL,
 };
 
+/* stock main() from FUSE example */
 int main(int argc, char *argv[])
 {
 	struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
diff --git a/dbfs.h b/dbfs.h
index 8f5bde1..6ebae72 100644
--- a/dbfs.h
+++ b/dbfs.h
@@ -32,12 +32,12 @@ struct dbfs_dirent {
 	guint16		namelen;
 	guint64		ino;
 	char		name[0];
-};
+} __attribute__ ((packed));
 
 struct dbfs_extent {
 	dbfs_blk_id_t	id;
 	guint64		size;
-};
+} __attribute__ ((packed));
 
 struct dbfs_raw_inode {
 	guint64		ino;
@@ -52,7 +52,7 @@ struct dbfs_raw_inode {
 	guint64		atime;
 	guint64		mtime;
 	struct dbfs_extent blocks[0];
-};
+} __attribute__ ((packed));
 
 struct dbfs_inode {
 	unsigned int		n_extents;