On Sun, 7 Jan 2007 23:12:56 -0500
"Josef 'Jeff' Sipek" <[EMAIL PROTECTED]> wrote:
> From: Josef "Jeff" Sipek <[EMAIL PROTECTED]>
>
> This patch contains helper functions used through the rest of the code which
> pertains to files.
>
> Signed-off-by: Josef "Jeff" Sipek <[EMAIL PROTECTED]>
> Signed-off-by: David Quigley <[EMAIL PROTECTED]>
> Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
>
> ...
>
> +#include "union.h"
> +
> +/* 1) Copyup the file
> + * 2) Rename the file to '.unionfs<original inode#><counter>' - obviously
> + * stolen from NFS's silly rename
> + */
> +static int copyup_deleted_file(struct file *file, struct dentry *dentry,
> + int bstart, int bindex)
> +{
> + static unsigned int counter;
> + const int i_inosize = sizeof(dentry->d_inode->i_ino) * 2;
> + const int countersize = sizeof(counter) * 2;
> + const int nlen = sizeof(".unionfs") + i_inosize + countersize - 1;
> + char name[nlen + 1];
> +
> + int err;
> + struct dentry *tmp_dentry = NULL;
> + struct dentry *hidden_dentry = NULL;
> + struct dentry *hidden_dir_dentry = NULL;
> +
> + hidden_dentry = unionfs_lower_dentry_idx(dentry, bstart);
Slightly strange to initialise a variable twice in a row like this.
> + sprintf(name, ".unionfs%*.*lx",
> + i_inosize, i_inosize, hidden_dentry->d_inode->i_ino);
> +
> + tmp_dentry = NULL;
> + do {
> + char *suffix = name + nlen - countersize;
> +
> + dput(tmp_dentry);
> + counter++;
> + sprintf(suffix, "%*.*x", countersize, countersize, counter);
> +
> + printk(KERN_DEBUG "unionfs: trying to rename %s to %s\n",
> + dentry->d_name.name, name);
> +
> + tmp_dentry = lookup_one_len(name, hidden_dentry->d_parent,
> + UNIONFS_TMPNAM_LEN);
> + if (IS_ERR(tmp_dentry)) {
> + err = PTR_ERR(tmp_dentry);
> + goto out;
> + }
> + } while (tmp_dentry->d_inode != NULL); /* need negative dentry */
> +
> + err = copyup_named_file(dentry->d_parent->d_inode, file, name, bstart,
> + bindex, file->f_dentry->d_inode->i_size);
> + if (err)
> + goto out;
> +
> + /* bring it to the same state as an unlinked file */
> + hidden_dentry = unionfs_lower_dentry_idx(dentry, dbstart(dentry));
> + hidden_dir_dentry = lock_parent(hidden_dentry);
> + err = vfs_unlink(hidden_dir_dentry->d_inode, hidden_dentry);
> + unlock_dir(hidden_dir_dentry);
> +
> +out:
> + return err;
> +}
> +
> +/* put all references held by upper struct file and free lower file pointer
> + * array
> + */
Where in this patchset would the reader go to understand what an "upper
file" is, what a "lower file" is? The relationship between them, lifecycle
management, locking, etc?
<looks at the data structures in union.h>
That's the place. It would be useful to describe things in there a lot
better. For example, bstart, bend and bindev could do with help.
unionfs_get_nlinks() is huge. it has twelve callsites and two out-of-line
copies are generated. Suggest that it not be inlined ;)
alloc_whname() should be out-of-line.
lock_parent() and unlock_dir() are poorly named.
> +long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + long err;
> +
> + if ((err = unionfs_file_revalidate(file, 1)))
> + goto out;
> +
> + /* check if asked for local commands */
> + switch (cmd) {
> + case UNIONFS_IOCTL_INCGEN:
> + /* Increment the superblock generation count */
> + err = -EACCES;
> + if (!capable(CAP_SYS_ADMIN))
> + goto out;
> + err = unionfs_ioctl_incgen(file, cmd, arg);
> + break;
> +
> + case UNIONFS_IOCTL_QUERYFILE:
> + /* Return list of branches containing the given file */
> + err = unionfs_ioctl_queryfile(file, cmd, arg);
> + break;
> +
> + default:
> + /* pass the ioctl down */
> + err = do_ioctl(file, cmd, arg);
> + break;
> + }
We normally use one tabstop less than this.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
