This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS


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
 > 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]