Re: [PATCH v5 3/4] fat: add renameat2 RENAME_EXCHANGE flag support

[Javier Martinez Canillas]

Hello OGAWA,

Thanks for your feedback.

On 6/9/22 21:17, OGAWA Hirofumi wrote:
> Javier Martinez Canillas <javierm@xxxxxxxxxx> writes:
> 
>> +static void vfat_update_nlink(struct inode *dir, struct inode *inode)
>> +{
>> +		if (S_ISDIR(inode->i_mode))
>> +			drop_nlink(dir);
>> +		else
>> +			inc_nlink(dir);
>> +}
> 
> [...]
> 
>> +	vfat_update_dir_metadata(old_dir, &ts);
>> +	/* if directories are not the same, update new_dir as well */
>> +	if (old_dir != new_dir) {
>> +		vfat_update_dir_metadata(new_dir, &ts);
>> +		/* nlink only needs to be updated if the file types differ */
>> +		if (old_inode->i_mode != new_inode->i_mode) {
>> +			vfat_update_nlink(old_dir, old_inode);
>> +			vfat_update_nlink(new_dir, new_inode);
>> +		}
>> +	}
> 
> Looks like unnecessary complex (and comparing raw i_mode, not S_ISDIR(),
> better to change before make dir dirty).  How about this change, it is
> only tested slightly though? Can you review and test?
>

Your change looks good to me and indeed the logic is simpler than in mine.

I've also tested it and AFAICT it works correctly as well. Do you plan to
squash this or should I respin a new revision of the whole patch-set ? If
you want to post it as a follow-up I'm also OK with that.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat