Hello!
On Tue 27-02-24 12:17:11, Dan Carpenter wrote:
> The patch ea554578483b: "ext4: do not create EA inode under buffer
> lock" from Feb 9, 2024 (linux-next), leads to the following Smatch
> static checker warning:
>
> fs/ext4/xattr.c:2265 ext4_xattr_ibody_set()
> warn: duplicate check 'error' (previous on line 2255)
>
> fs/ext4/xattr.c
> 2232 int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
> 2233 struct ext4_xattr_info *i,
> 2234 struct ext4_xattr_ibody_find *is)
> 2235 {
> 2236 struct ext4_xattr_ibody_header *header;
> 2237 struct ext4_xattr_search *s = &is->s;
> 2238 struct inode *ea_inode = NULL;
> 2239 int error;
> 2240
> 2241 if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
> 2242 return -ENOSPC;
> 2243
> 2244 /* If we need EA inode, prepare it before locking the buffer */
> 2245 if (i->value && i->in_inode) {
> 2246 WARN_ON_ONCE(!i->value_len);
> 2247
> 2248 ea_inode = ext4_xattr_inode_lookup_create(handle, inode,
> 2249 i->value, i->value_len);
> 2250 if (IS_ERR(ea_inode))
> 2251 return PTR_ERR(ea_inode);
> 2252 }
> 2253 error = ext4_xattr_set_entry(i, s, handle, inode, ea_inode,
> 2254 false /* is_block */);
> 2255 if (error) {
> ^^^^^
>
> 2256 if (ea_inode) {
> 2257 int error2;
> 2258
> 2259 error2 = ext4_xattr_inode_dec_ref(handle, ea_inode);
> 2260 if (error2)
> 2261 ext4_warning_inode(ea_inode, "dec ref error=%d",
> 2262 error2);
> 2263
> 2264 /* If there was an error, revert the quota charge. */
> --> 2265 if (error)
> ^^^^^
> We know "error" is non-zero. I'm not sure whether to delete this check
> or change "error" to "error2".
Deleting the check is the right solution. The patch didn't go upstream in
the end anyway for now because it has some functional issues but I've fixed
this up locally. Thanks for report!
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
