Re: [RFA] deleting breakpoints inside of 'commands' [Repost]

[Fernando Nasser <fnasser@redhat.com>]

Kevin Buettner wrote:
> 
> On Sep 26,  4:01pm, Fernando Nasser wrote:
> 
> > And I don't think Don's latest patch is complex, or considerably more
> > complex than the simplistic copy approach (or hack!).  It is elegant
> > (although Kevin's comments are valid and should be incorporated).
> >
> > If Don can add a cleanup function and do the polishing suggested by Kevin
> > on his last patch I suggest that we stick with that one.
> 
> Here are my preferences...
> 
> I like the patch that makes a copy the best since the changes are easy
> to understand and localized to one function.
> 
> Next best would be a patch (unwritten as of now) which implements
> reference counts for these command chains.  I think that a lot of
> folks understand reference counts and it takes less time to grok code
> written with this design pattern.  In all likelyhood, such a patch
> would look somewhat similar to Don's latest patch though; i.e, the
> changes would be distributed among several files and would likely be
> in the same functions that Don has touched with his latest patch.
> 
> Next best would be Don's latest patch revised to use an enum for
> the ``execute'' member.  I think this code would be easier to
> understand by giving the 0, 1, and 2+ values actual names that are
> meaningful.  I still have concerns about what might happen if
> bpstat_do_actions() is ever executed recursively...  I don't think this
> ever happens, so it's probably a non-issue, but if it could happen,
> then a reference counting mechanism would definitely be superior.
> 
> (All of the above approaches need to make use of a cleanup.)
> 
> Kevin
> 

That is a good analysis.  My preference is know to be the second above.
However, if a _real_ consensus is reached on the external list that we
should just use the duplicating patch I will remove my objections.


> P.S. I'd like to publicly thank Don for all the hard work he has
> put in on this patch.  I imagine it must be frustrating to be told
> to do the patch one way by one person, a different way by a second,
> and still a different way by a third...

I would like to second that.  It must be noted that Don has not, in 
any moment, has complained of anything.  On the contrary, he kept
thinking about the problem and trying to provide alternative 
solutions.  In the best tradition of the best Open Source contributors.

W.r.t. the pain of having to re-work patches comes with the job.  That
people will have different views and preferences comes with humanity.
This happens in any Open Source project -- we have been through this 
before in this list and the same thing happens daily on the PostgreSQL
database project lists.  We could have discussed alternatives without
Don writing patches, but it was his patches that kept bringing more
light into the problem (fr himself included).   And he learned that
globals are no-no's among other things, was able to understand some
more parts of the code etc.  It was not wasted time.

P.S.: I would like to see some people going through a Ph.D. and having
to change things in circle until your supervisor understands you. 


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9