>>>>> Andrew Morton (AM) writes:
>> From a quick scan:
AM> - The code is very poorly commented. I'd want to spend a lot of time
AM> reviewing this implementation, but not in its present state.
what sort of comments are you expecting?
AM> - Far, far too many inlines
probably, I'll review the code in this regard
AM> - overly-terse variable naming
same
AM> - There are several places which appear to be putting block numbers into
AM> an `int'.
same
AM> - Needs kmalloc()->kzalloc() conversion
OK
AM> - replace all brelse() calls with put_bh(). Because brelse() is
AM> old-fashioned, has a weird name and neelessly permits a NULL arg.
AM> In fact it would be beter to convert JBD and ext3 to put_bh before
AM> copying it all over.
OK
AM> - The open-coded __clear_bit(BH_New, ...) in ext4_ext_get_blocks is a bit
AM> nasty. We can live with nasty, but are we sure that it isn't buggy??
I believe it isn't buggy -- it applies to non-shared var.
it also showed minor improvement on SMP.
AM> - It has about 7,000 instances of
AM> if ((lhs = expression)) {
AM> whereas the preferred coding style is
AM> lhs = expression;
AM> if (lhs) {
OK
AM> - The existing comments could benefit from some rework by a native English
AM> speaker.
could someone assist here, please?
thanks, Alex
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
