On Fri, Oct 29, 2010 at 7:33 PM, Marti Raudsepp <ma...@juffo.org> wrote: > Since my previous Coccinelle cleanup patches have gained positive > feedback I decided to try some more. > > This time I wrote my own spatches. > > patch 0001 turns (a - b == 0) into (a == b) and similarly with != > patch 0002 applies the same to operators >, >=, <, <= > > I'm well aware that there's a point where code cleanups defeat their > purpose and become a burden. So this will probably be my last one, > I'll go to doing productive things instead. :)
I like the 0002 patch better than the 0001 patch. For example, consider: - while (q - (uint8 *) keybuf - 8) + while (q - (uint8 *) keybuf != 8) That's pretty darn inscrutable either way, and the loop that follows does absolutely nothing to clear anything up. What's so special about 8? Are we counting up to 8 or down to 8? Most of these examples involve an additive constant, or (in the case of zic.c) are parallel to surrounding code that does similar tests with an additive constant. I'm pretty much inclined to think that these examples should either be left alone or given a more substantial face lift. On the other hand, most of the stuff in the 0002 patch takes this form: - if (p - tmpbuf > 0) + if (p > tmpbuf) IMHO, that's an improvement. However, I think that this rewrite changes the overflow behavior. There's a comment not too far from the bgwriter.c change that seems to imply that the test is written the way that it is specifically to produce a certain overflow behavior. The others all look OK, though, as far as I can tell. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
