apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin <share-apa...@think42.com>
Subject Re: various atomic operations
Date Tue, 24 Oct 2006 08:15:20 GMT
On Mon, Oct 23, 2006 at 10:22:03AM +0100, Joe Orton wrote:
> On Fri, Oct 20, 2006 at 05:35:47PM +0200, Colin wrote:
> > On Thu, Oct 19, 2006 at 05:04:12PM -0700, Garrett Rooney wrote:
> > > On 10/19/06, Colin <share-apache@think42.com> wrote:
> > > >Hi Again,
> > > >
> > > >I have finally found a few minutes to write down all issues that I
> > > >found in apr_atomic.c ... I would now like to know whether, and
> > > >for which of the points, there is interest in further discussion
> > > >and/or finally a patch.
> > > 
> > > If you actually have patches to correct whatever problems there are,
> > > then great, send them in.  You're considerably more likely to get a
> > > response from an actual patch (preferably that fixes one specific
> > > problem) than by just describing the problem.
> > 
> > Ok ... below is the output of 'svn diff', and attached is a .tgz with
> > the diff and the two changed files. Nothing else was touched yet, as
> > it is probably better to wait which changes actually get accepted.
> > Please feel free to ask about any details of the changes... A brief
> > list of changed points, and other relevant notes, is at the head of
> > the .c file.
> 
> The emphasis should have been on patch*es* in the plural there!  When 
> you're making multiple sets of changes, it's better to send one patch 
> per functional change (or set of closely-related changes, possibly), so 
> they can be reviewed separately.
> 
> Without splitting out whitespace changes, non-functional changes, code 
> re-ordering etc this is very hard to review.  Also, the function 
> prototype changes need to be addressed separately because they can't be 
> used in any 1.x release.
> 
> Would it be possible for you to split this out and resend it?  e.g. one 
> patch for Solaris-specific fixes, one for the Darwin implemention, etc. 
> Also - the changes made should be described in the mail with which each 
> patch is sent, not in the source file itself.

My intention was to use the files I sent in the last mail as starting point
for further discussion; the hope is that looking at the original file and
the changed file with the comments allows for a first dialog, and perhaps
the decision on whether a change would be accepted.

(The list of changes has about 20 items, and I tested 5 versions on 3
machines, so I would need to create 20 patches and make 100 tests, but
if one of the first, more global changes gets rejected I end up with
having to re-do most of them ... that's why I would like to pre-filter.)

For example, if it is felt that the global moving around of functions,
e.g. creating dedicated NOTHREADS implementations, is not wanted, I 
would omit that part.

Other places that were in my opinion broken, like the wrong Solaris 10 
function calls, could be more or less nodded of immediately.

Perhaps we can start this way, and then I can start feeding small patches,
that are intended for inclusion, and only make one change... Even if you
just tell me where to start.

Regards, Colin



Mime
View raw message