Re: [PATCH 16/35] Unionfs: Copyup Functionality

[Josef Sipek <jsipek@fsl.cs.sunysb.edu>]

On Tue, Dec 05, 2006 at 10:09:19PM +0100, Jan Engelhardt wrote:
> 
> On Dec 4 2006 07:30, Josef 'Jeff' Sipek wrote:
> >+/* Determine the mode based on the copyup flags, and the existing dentry. */
> >+static int copyup_permissions(struct super_block *sb,
> >+			      struct dentry *old_hidden_dentry,
> >+			      struct dentry *new_hidden_dentry)
> >+{
> >+	struct inode *i = old_hidden_dentry->d_inode;
> 
> Screams for constification. (Or rather I do.)
 
I'm not following.
 
> >+{
> >+	int err = 0;
> >+	umode_t old_mode = old_hidden_dentry->d_inode->i_mode;
> 
> Generel question for everybody: Why do we have two same (at least on i386
> that's the case) types, umode_t and mode_t?

No idea.

> >+	} else if (S_ISBLK(old_mode)
> >+		   || S_ISCHR(old_mode)
> >+		   || S_ISFIFO(old_mode)
> >+		   || S_ISSOCK(old_mode)) {
> >+		args.mknod.parent = new_hidden_parent_dentry->d_inode;
> >+		args.mknod.dentry = new_hidden_dentry;
> >+		args.mknod.mode = old_mode;
> 
> I'd say the indent got screwed up, and it's not a mailer thing.

Right.

> >+	input_file = dentry_open(old_hidden_dentry,
> >+			unionfs_lower_mnt_idx(dentry, old_bindex), O_RDONLY | O_LARGEFILE);
> >+	if (IS_ERR(input_file)) {
...
> >+	output_file = dentry_open(new_hidden_dentry,
> >+			unionfs_lower_mnt_idx(dentry, new_bindex), O_WRONLY | O_LARGEFILE);
> 
> Here we got an 80-column buster.

/me whistles innocently

> >+	/* TODO: should we reset the error to something like -EIO? */
> 
> Handle it :)  - if it does not take a paper.

I'm not even sure if we want to reset it to begin with.

If we don't reset, the user may get some non-sensical errors, but on the
other hand, if we reset to EIO, we guarantee that the user will get a
"confusing" error message.

Josef "Jeff" Sipek.

-- 
All science is either physics or stamp collecting.
		- Ernest Rutherford
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/