[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: linked list library

From: John Darrington
Subject: Re: linked list library
Date: Sat, 1 Jul 2006 07:29:46 +0800
User-agent: Mutt/1.5.4i

On Thu, Jun 29, 2006 at 09:23:18PM -0700, Ben Pfaff wrote:
     I added a patch containing a linked list implementation that I
     would like to check in to libpspp.  I'd appreciate some review,
     if you guys have time:

It seems to have everything that a linked list could want (and more!).

1.  Some of the headers have got the 59 Temple Place address.

2.  I ran gcov on the tests.  All lines of code are indeed exercised,
    but not quite all branches.  I don't know if you're worried about that.

3.  The OFFSETOF macro seems to be a GNU builtin.  So far as I'm
    aware, it's not part of any C standard.  Will this be portable
    enough?  Maybe it's in gnulib; I haven't looked.

4.  It might be useful to have a const version of ll[x]_apply.

5.  How about an assertion to ensure the requirement that
    llx_set_allocator is called before any other llx_ function ?

     By the way, I've decided that it's time to start getting code
     reviews of nontrivial changes to PSPP, as is done in some other
     GNU projects (not to mention commercial development), so that's
     why I'm posting these patches.

OK.   This raises some questions in my mind:

1.  How do I distinguish between a trivial and nontrivial change ?

2.  What's the protocol for getting reviews done?  Post them as
    patches, and put out a general "request for review" call on
    pspp-dev ?  Who should perform the review ?  For example, in the
    case of changes to the GUI code, there doesn't seem to be any
    developer who is currently sufficiently familiar to review the

3.  How do I know when a review has been approved?  Will we have an
    approval keyword?

The "code reviews" conducted at any commercial organisations with
which I've been involved, have been a joke.   Nobody had the
time/competance/inclination to do them.  Consequently, if there were
done at all, they were simply a beaurocratic exercise.


PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See or any PGP keyserver for public key.

Attachment: pgp4F_KyAstLh.pgp
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]