Hi Alvaro, Thanks for the review!
On Thursday, September 06, 2012 06:09:35 PM Alvaro Herrera wrote: > Here's a prettified version of this stuff. I found one bug in the macro > ilist_s_head: the test was reversed. Oh, good catch. I had only used the _unchecked version because my code checked that there are elements just some lines before that... > Also, curiously, the macro had the same name as the struct, so I renamed the > macro. I take it you haven't used this macro, so maybe it shouldn't be there at all? Or maybe I completely misread what the macro is supposed to do. According to my patches here that got introduced by me whe renaming _front/back to _head/tail according to Roberts wishes. Sorry for that. > I also renamed all the structs and functions by changing ilist_s_foo to > Slist_foo. Similarly for ilist_d_foo. This is all mechanical so any > subsequent patch should be trivial to refresh for this change. Ok. I concur with robert that a lower case first letter might be better readable but again, I don't really care that much. > I think README.ilist (which is what you had in the comment at the top of > ilist.h) should be heavily expanded. I don't find it at all clear. Hm. I agree :(. Let me have a go when you have a state you find acceptable otherwise... > There were other cosmetic changes, but the implementation is pretty much > the same you submitted. Good. > I didn't look at the other patch you posted, replacing dllist.c usage; > will do that next to verify that the list implementation works. Thanks! Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
