From mboxrd@z Thu Jan 1 00:00:00 1970 From: Elena Zannoni To: Andrew Cagney Cc: Daniel Jacobowitz , Elena Zannoni , gdb-patches@sources.redhat.com Subject: Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS Date: Fri, 12 Oct 2001 09:05:00 -0000 Message-id: <15303.5768.383661.564989@krustylu.cygnus.com> References: <20011009123457.A28534@nevyn.them.org> <15302.12121.778853.77938@krustylu.cygnus.com> <20011011195450.A22256@nevyn.them.org> <3BC63C81.5000102@cygnus.com> X-SW-Source: 2001-10/msg00170.html Andrew Cagney writes: > > > > OK. Would you prefer I resubmit this patch broken up further, then? > > I could do that. > > > > There's a double-edged sword here; every patch in this sequence except > > for the hashing change is predicated on the previous patches. So while > > I understand that breaking them up does make reviewing much easier, > > with the current length of the patch review cycle, every time I > > decompose this further I add two or three more days to its eventual > > (hopeful) approval. I'm sure you can understand that it's a little > > frustrating. > > Daniel, > > As you said, it is a double-edged sword. The other edge has a very > unusual feature. Identify simple mechanical self contained changes and > often go in as obvious. The review cycle goes down and can often be > reduced to zero. > > While this can mean an increased workload for you as an individual it > does dramatically reduce the work load for the entire GDB community. > > My reading of Elena's comment: > > > Yes, I looked ths over and it seems to work, except that I would really > > prefer the change to printcmd.c split in two. The first bit to > > rationalize that "if (func)..." code. This would have with it all > > the indentation changes as well. The code as it is now doesn't really > > make much sense. So, that looks a good change to me. But it has nothing > > to do with the new macro. After that change is in, you can introduce > > the macro in printcmd.c w/o having all the indent changes. > > It also makes it easier to distinguish a no-op change (the macro) from > > the other one. > > is that you're all approved. Yes, sorry if that wasn't clear. > > Your first commit fixes some messed up logic. It is a cleanup (but > pretty obvious). It doesn't have anything to do with the (ULGH) macro. > By keeping it separate it makes it possible to better isolate the > breakage it could cause when we have to go back (in 6 months) to find a > bug ;-) > > Your second commit is this new (ULGH) macro. The macro (ULGH) shouldn't > break anything but it is however still a (ULGH) macro. Just include the > extra tweeks you found. > Yes. Elena > (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro > so I'm biteing my tongue on this change :-) > > Andrew >