Ha, disregard, caught up to read the other email and I see everyone else got there too. I would've been in the thread an hour ago, but unfortunately timed call back from Microsoft support, much to my dismay. Carry on then... -----Original Message----- From: xfs-bounces@xxxxxxxxxxx [mailto:xfs-bounces@xxxxxxxxxxx] On Behalf Of Shaun Gosse Sent: Friday, February 28, 2014 3:33 PM To: Mark Tinguely; Brian Foster Cc: xfs@xxxxxxxxxxx Subject: RE: [PATCH] xfs: check all buffers in xfs_check_page_type() > Is there any reason to scan all the buffers when we all we want is an indication that at least one is acceptable? > Maybe there are generally not may buffers to a page to make it worthwhile. > The problem is the else doesn't check if acceptable is set before breaking. So it can quit early, not having found what it was looking for even though it was in one of the later buffers. Pre-mature optimization... So the patch is good as is. It improves correctness. If the optimization were desired, though, it could be: + else if (acceptable) + break In addition to the remove already posted, and with proper whitespace. Fair warning: this is without looking at the code, just inferring from what's been written in those two emails. I'm hearing my CS101 professor in my head and thinking may not be worth a couple cycles at the risk of correctness or clarity (and suggesting code blind seems risky). Actually, this could be the first condition. If the whole point of that loop is just to make sure to set acceptable if it can be set according to the rules on the basis of at least one of the buffers, then a starting: + if (acceptable) + break With an annotation something like "/** Optimization: Quit when acceptable has been set. Loop has no other side-effects to worry about.*/" seems reasonable (and hopefully is true). And then it's clear from the code that's the point of the loop and how it works. Cheers, -Shaun _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
