Re: 2.6.29 -mm merge plans

From: Diego E. 'Flameeyes'
Date: Tue Jan 06 2009 - 19:09:43 EST


On Tue, 2009-01-06 at 18:31 -0500, Christoph Hellwig wrote:
> Just some minor comments:

I'm attaching a version fixing both yours comments and Harvey's.

> Please don't put filenames in top of file comments. They don't serve
> any purpose and easily get out of date.

I've done that just to make it "fit-in" with the rest of the hfsplus
code, should I send a patch to remove those from the other files?

> No need for this comment I think. All this is pretty well documented
> in Documentation/filesystems/Exporting, and all the other filesystems
> that just fill out these three methods don't comment on it either.

I copied over the code from ext2 sources, so at least one other
filesystem does comment on it ;)

But indeed it's misleading, better for it to be gone.

On Tue, 2009-01-06 at 15:49 -0800, Harvey Harrison wrote:
> One other nit, byteswap the constant so it can be done at compile-time:
>
I copied over the code from dir.c, so I've propagated the change to
that, and also to super.c where a similar case was present, I'm
attaching it at 0002 (but maybe it should go in before the NFS export
support?).

I've not checked if there are other cases where this can be optimised
though, maybe I should.

If you all are in mood of HFS+ patches review, I might try to run the
code through a couple of my tools, I had in my TODO list to try them on
kernel code for a while ;)

--
Diego "Flameeyes" PettenÃ
http://blog.flameeyes.eu/

From 854146b9bc0143e9e3bffc765388b9dacb490527 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Diego=20E.=20'Flameeyes'=20Petten=C3=B2?= <flameeyes@xxxxxxxxx>
Date: Thu, 4 Dec 2008 13:32:06 +0100
Subject: [PATCH] Add basic export support to HFS+ filesystem.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

The functions' skeleton is taken from ext2/super.c and seems to work
fine for R/W access to HFS+ non-journaled case-sensitive filesystems.

Signed-off-by: Diego E. 'Flameeyes' Pettenà <flameeyes@xxxxxxxxx>
Cc: Roman Zippel <zippel@xxxxxxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Neil Brown <neilb@xxxxxxx>
---
fs/hfsplus/Makefile | 3 +-
fs/hfsplus/export.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++
fs/hfsplus/hfsplus_fs.h | 3 +
fs/hfsplus/super.c | 1 +
4 files changed, 110 insertions(+), 1 deletions(-)
create mode 100644 fs/hfsplus/export.c

diff --git a/fs/hfsplus/Makefile b/fs/hfsplus/Makefile
index 3cc0df7..374131e 100644
--- a/fs/hfsplus/Makefile
+++ b/fs/hfsplus/Makefile
@@ -5,5 +5,6 @@
obj-$(CONFIG_HFSPLUS_FS) += hfsplus.o

hfsplus-objs := super.o options.o inode.o ioctl.o extents.o catalog.o dir.o btree.o \
- bnode.o brec.o bfind.o tables.o unicode.o wrapper.o bitmap.o part_tbl.o
+ bnode.o brec.o bfind.o tables.o unicode.o wrapper.o bitmap.o part_tbl.o \
+ export.o

diff --git a/fs/hfsplus/export.c b/fs/hfsplus/export.c
new file mode 100644
index 0000000..3486e54
--- /dev/null
+++ b/fs/hfsplus/export.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) 2001
+ * Brad Boyer (flar@xxxxxxxxxxxxx)
+ * (C) 2003 Ardis Technologies <roman@xxxxxxxxxxxxx>
+ * (C) 2008 Diego E. Pettenà <flameeyes@xxxxxxxxx>
+ *
+ * Export for NFS
+ */
+
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/exportfs.h>
+
+#include "hfsplus_fs.h"
+#include "hfsplus_raw.h"
+
+/*
+ * This is very different from most get_parent functions, since HFS+
+ * does not have a ".." entry on their directories.
+ *
+ * Instead, the filesystem uses Catalog Thread Records to connect
+ * directories and files to their ancestors.
+ */
+static struct dentry *hfsplus_get_parent(struct dentry *child)
+{
+ struct super_block *sb;
+ hfsplus_cat_entry entry;
+ struct hfs_find_data fd;
+ struct inode *inode;
+ int err;
+
+ sb = child->d_sb;
+
+ hfs_find_init(HFSPLUS_SB(sb).cat_tree, &fd);
+ hfsplus_cat_build_key(sb, fd.search_key, child->d_inode->i_ino, NULL);
+ err = hfs_brec_find(&fd);
+
+ if (err) {
+ printk(KERN_ERR "hfs: unable to find child, call police\n");
+ goto out;
+ }
+
+ hfs_bnode_read(fd.bnode, &entry, fd.entryoffset, fd.entrylength);
+ if (entry.type != cpu_to_be16(HFSPLUS_FOLDER_THREAD)) {
+ printk(KERN_ERR "hfs: bad catalog folder thread\n");
+ err = -EIO;
+ goto out;
+ }
+
+ if (fd.entrylength < HFSPLUS_MIN_THREAD_SZ) {
+ printk(KERN_ERR "hfs: truncated catalog thread\n");
+ err = -EIO;
+ goto out;
+ }
+
+ hfs_find_exit(&fd);
+
+ inode = hfsplus_iget(child->d_sb, be32_to_cpu(entry.thread.parentID));
+ if (IS_ERR(inode)) {
+ printk(KERN_ERR
+ "hfs: unable to find parent, call social services\n");
+ return ERR_CAST(inode);
+ }
+
+ return d_obtain_alias(inode);
+
+out:
+ hfs_find_exit(&fd);
+ return ERR_PTR(err);
+}
+
+static struct inode *hfsplus_export_get_inode(struct super_block *sb,
+ u64 ino, u32 generation)
+{
+ struct inode *inode;
+
+ if (ino < HFSPLUS_FIRSTUSER_CNID && ino != HFSPLUS_ROOT_CNID)
+ return ERR_PTR(-ESTALE);
+
+ /* hfsplus doesn't set i_generation so no need to check for it. */
+ return hfsplus_iget(sb, ino);
+}
+
+static struct dentry *hfsplus_fh_to_dentry(struct super_block *sb, struct fid *fid,
+ int fh_len, int fh_type)
+
+{
+ return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
+ hfsplus_export_get_inode);
+}
+
+static struct dentry *hfsplus_fh_to_parent(struct super_block *sb, struct fid *fid,
+ int fh_len, int fh_type)
+
+{
+ return generic_fh_to_parent(sb, fid, fh_len, fh_type,
+ hfsplus_export_get_inode);
+}
+
+const struct export_operations hfsplus_export_ops = {
+ .get_parent = hfsplus_get_parent,
+ .fh_to_dentry = hfsplus_fh_to_dentry,
+ .fh_to_parent = hfsplus_fh_to_parent,
+};
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index f027a90..7c78525 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -371,6 +371,9 @@ int hfsplus_read_wrapper(struct super_block *);

int hfs_part_find(struct super_block *, sector_t *, sector_t *);

+/* export.c */
+extern const struct export_operations hfsplus_export_ops;
+
/* access macros */
/*
static inline struct hfsplus_sb_info *HFSPLUS_SB(struct super_block *sb)
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index eb74531..b5d1153 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -345,6 +345,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)

/* Set up operations so we can load metadata */
sb->s_op = &hfsplus_sops;
+ sb->s_export_op = &hfsplus_export_ops;
sb->s_maxbytes = MAX_LFS_FILESIZE;

if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
--
1.6.0.6

From 3b47da5b18b4463d7d146534182c40b95b6ca6d4 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Diego=20E.=20'Flameeyes'=20Petten=C3=B2?= <flameeyes@xxxxxxxxx>
Date: Wed, 7 Jan 2009 01:02:37 +0100
Subject: [PATCH] Swap the constant when comparing values read from disk.

This way the swap can be done at build-time without doing it on the running
system.

Upon comment by Harvey Harrison.
---
fs/hfsplus/dir.c | 2 +-
fs/hfsplus/super.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 5f40236..16f825b 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -139,7 +139,7 @@ static int hfsplus_readdir(struct file *filp, void *dirent, filldir_t filldir)
/* fall through */
case 1:
hfs_bnode_read(fd.bnode, &entry, fd.entryoffset, fd.entrylength);
- if (be16_to_cpu(entry.type) != HFSPLUS_FOLDER_THREAD) {
+ if (entry.type != cpu_to_be16(HFSPLUS_FOLDER_THREAD)) {
printk(KERN_ERR "hfs: bad catalog folder thread\n");
err = -EIO;
goto out;
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index b5d1153..7ef993b 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -182,7 +182,7 @@ static void hfsplus_write_super(struct super_block *sb)
bh = sb_bread(sb, block);
if (bh) {
vhdr = (struct hfsplus_vh *)(bh->b_data + offset);
- if (be16_to_cpu(vhdr->signature) == HFSPLUS_VOLHEAD_SIG) {
+ if (vhdr->signature == cpu_to_be16(HFSPLUS_VOLHEAD_SIG)) {
memcpy(vhdr, HFSPLUS_SB(sb).s_vhdr, sizeof(*vhdr));
mark_buffer_dirty(bh);
brelse(bh);
--
1.6.0.6

Attachment: signature.asc
Description: This is a digitally signed message part