Re: [PATCH v4 10/13] exfat: add nls operations

[Markus Elfring]

â
> +++ b/fs/exfat/nls.c
â
> +static int exfat_load_upcase_table(struct super_block *sb,
> +		sector_t sector, unsigned long long num_sectors,
> +		unsigned int utbl_checksum)
> +{
â
> +	int ret = -EIO;
â
> +	while (sector < num_sectors) {
> +		bh = sb_bread(sb, sector);
> +		if (!bh) {
> +			exfat_msg(sb, KERN_ERR,
> +				"failed to read sector(0x%llx)\n", sector);
> +			goto release_bh;
> +		}
â
> +	}
> +
> +	if (index >= 0xFFFF && utbl_checksum == checksum) {
> +		brelse(bh);
> +		return 0;
> +	}
> +
> +	exfat_msg(sb, KERN_ERR,
> +			"failed to load upcase table (idx : 0x%08x, chksum : 0x%08x, utbl_chksum : 0x%08x)\n",
> +			index, checksum, utbl_checksum);
> +
> +	ret = -EINVAL;

Can a blank line be omitted between the message and the error code?


> +release_bh:
> +	brelse(bh);
> +	exfat_free_upcase_table(sb);
> +	return ret;
> +}

I got the impression that the resource management is still questionable
for this function implementation.

1. Now I suggest to move the call of the function âbrelseâ to the end
   of the while loop. The label ârelease_bhâ would be renamed to âfree_tableâ then.

2. Can a variable initialisation be converted to the assignment âret = -EIO;â
   in an if branch?

Regards,
Markus