Messages in this thread | | | Subject | Re: [PATCH v4 10/13] exfat: add nls operations | From | Markus Elfring <> | Date | Thu, 21 Nov 2019 09:09:10 +0100 |
| |
… > +++ 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
| |