From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Fri, 18 Nov 2011 21:28:10 +0000 Subject: [Cluster-devel] [PATCH] libgfs2: Improve rgblocks2bitblocks() In-Reply-To: <15229912-62e2-480b-ab7e-672d89e3aa2f@zmail16.collab.prod.int.phx2.redhat.com> References: <15229912-62e2-480b-ab7e-672d89e3aa2f@zmail16.collab.prod.int.phx2.redhat.com> Message-ID: <4EC6CDEA.4060603@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 18/11/11 20:52, Bob Peterson wrote: > ----- Original Message ----- > | This patch reworks the rgblocks2bitblocks function which was > | inefficient, difficult to read and generally unwieldy. > | > | As this is core code from the days of yore and fsck.gfs2 depends on > | it, > | I made sure to test the new function extensively, comparing its > | outputs > | with the original function over a large range of values for rgblocks > | (up > | to 195312500) and valid block sizes between 512 and 4096. > | > | All call points have been updated and, as a nice side effect, the run > | time of the function is greatly reduced. > | > | Signed-off-by: Andrew Price > | + while (blks_rgrp + blks_meta * bitblocks< ((rgblocks - bitblocks) > > Hi, > > The patch looks good. There's only thing I'd do differently: > I know the implied arithmetic operator order is correct, but > I still prefer to see parens around statements like the above > just for clarity. e.g. > > + while (blks_rgrp + (blks_meta * bitblocks)< ((rgblocks - bitblocks) > > I guess that's more of a style thing. Yeah, it is more readable that way, I'll change it. Andy