Re: [PATCH 2/6] shmem: make shmem_get_inode() return ERR_PTR instead of NULL

From: Jan Kara <jack@suse.cz>
To: cem@kernel.org
Cc: hughd@google.com, jack@suse.cz, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org, djwong@kernel.org
Subject: Re: [PATCH 2/6] shmem: make shmem_get_inode() return ERR_PTR instead of NULL
Date: Mon, 3 Apr 2023 12:23:54 +0200	[thread overview]
Message-ID: <20230403102354.jnwrqdbhpysttkxm@quack3> (raw)
In-Reply-To: <20230403084759.884681-3-cem@kernel.org>

On Mon 03-04-23 10:47:55, cem@kernel.org wrote:
> From: Lukas Czerner <lczerner@redhat.com>
> 
> Make shmem_get_inode() return ERR_PTR instead of NULL on error. This will be
> useful later when we introduce quota support.
> 
> There should be no functional change.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

One comment below:

> @@ -2958,21 +2967,24 @@ shmem_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
>  	      struct file *file, umode_t mode)
>  {
>  	struct inode *inode;
> -	int error = -ENOSPC;
> +	int error;
>  
>  	inode = shmem_get_inode(idmap, dir->i_sb, dir, mode, 0, VM_NORESERVE);
> -	if (inode) {
> -		error = security_inode_init_security(inode, dir,
> -						     NULL,
> -						     shmem_initxattrs, NULL);
> -		if (error && error != -EOPNOTSUPP)
> -			goto out_iput;
> -		error = simple_acl_create(dir, inode);
> -		if (error)
> -			goto out_iput;
> -		d_tmpfile(file, inode);
> -	}
> +
> +	if (IS_ERR(inode))
> +		return PTR_ERR(inode);

This doesn't look correct. Previously, we've called
finish_open_simple(file, error), now you just return error... Otherwise the
patch looks good to me.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR