Re: [RFC] persistent store (version 3) (part 1 of 2)

[Borislav Petkov <bp@alien8.de>]

On Thu, Dec 09, 2010 at 11:12:02AM -0800, Luck, Tony wrote:
> > This upsets the traditional layout of having the error
> > recovery part of the function undo all the things that
> > we did leading up to the error. Pity, because your
> > version is easier to read.
> 
> But most of your "move the error fixups to the tail, so the
> normal code path is easier to follow" bit do still hold and
> can be used.  How about this:
> 
> -Tony
> 
> ---
> 
> int pstore_mkfile(char *name, char *data, size_t size, struct timespec time,
> 		  void *private)
> {
> 	struct dentry	*root = pstore_sb->s_root;
> 	struct dentry	*dentry;
> 	struct inode	*inode;
> 	int		rc;
> 
> 	rc = -ENOMEM;
> 	inode = pstore_get_inode(pstore_sb, root->d_inode, S_IFREG | 0444, 0);
> 	if (!inode)
> 		goto fail;

I'd add a newline here.

> 	inode->i_private = private;
> 
> 	mutex_lock(&root->d_inode->i_mutex);
> 
> 	rc = -ENOSPC;
> 	dentry = d_alloc_name(root, name);
> 	if (IS_ERR(dentry))
> 		goto fail_alloc;

ditto.

> 	d_add(dentry, inode);
> 
> 	mutex_unlock(&root->d_inode->i_mutex);
> 
> 	if (!pstore_writefile(inode, dentry, data, size))
> 		goto fail_write;
> 
> 	if (time.tv_sec)
> 		inode->i_mtime = inode->i_ctime = time;
> 
> 	return 0;
> 
> fail_write:
> 	inode->i_nlink--;
> 	mutex_lock(&root->d_inode->i_mutex);
> 	d_delete(dentry);
> 	dput(dentry);
> 	mutex_unlock(&root->d_inode->i_mutex);
> 	goto fail;

Yeah, either that or add a "return rc" to save us the jmp. But since
those paths wouldn't be hit in the normal case, I don't think it matters
that much.

> 
> fail_alloc:
> 	mutex_unlock(&root->d_inode->i_mutex);
> 	iput(inode);
> 
> fail:
> 	return rc;

Thanks.

-- 
Regards/Gruss,
    Boris.