[Top][All Lists]

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

Re: ROC is ready for review

From: John Darrington
Subject: Re: ROC is ready for review
Date: Mon, 20 Jul 2009 04:41:50 +0000
User-agent: Mutt/1.5.18 (2008-05-17)

I pushed a few changes addressing these last points.

On Sun, Jul 19, 2009 at 02:56:43PM -0700, Ben Pfaff wrote:
     John Darrington <address@hidden> writes:
     > On Sat, Jul 18, 2009 at 09:39:38PM -0700, Ben Pfaff wrote:
     >      I don't really understand the algorithm used by
     >      casereader_create_distinct.  Why are there two layers
     >      (casereader_create_filter_func and casereader_create_translator)?
     > Ease of implementation I think.
     >      Why is it necessary to use casereader_peek (which forces all of
     >      the input data to stay around) instead of reading cases one at a
     >      time (and possibly being able to throw away data that has been
     >      consumed)?
     > I couldn't find any other solution which actually worked.
     OK.  I think that this can be improved, but it doesn't have to be
     for now.
     >      process_group does a *lot* of manipulation of cases and
     >      caseprotos and readers and writers.  I didn't check it in detail
     >      because it wasn't obvious to me what the overall goal was.  It
     >      could really use a high-level comment.
     > I added a comment which explains that it actually does 3 things. In
     > order to keep down the number of data passes, it's necessary to have this
     > multi-purpose function.
     Thanks for adding the comments.
     In "Respect the constness of caseproto." I don't understand the
     benefit of the new caseproto_clone function.  As far as I can
     tell there is never any benefit in calling it, because it is
     always equivalent to simply calling caseproto_ref, except that
     with caseproto_clone the caller doesn't get the benefit of lazy
     reallocation or lazy refreshing of the long string cache.
     Maybe this property of caseproto_ref could be made clearer if
     caseproto_clone were simply implemented as "return caseproto_ref
     (Note that this property is true only because every function that
     modifies a caseproto returns a new caseproto that replaces it.
     Otherwise we'd need a public caseproto_unshare function, like
     case_unshare, and functions that modify caseprotos would only
     accept caseprotos with refcount 1.)
     I still don't see anything that unrefs cdr->proto when the
     casereader is destroyed.  Does it happen somewhere that I'm not
     In "Fix cleanup of ROC command." all of the added goto statement
     have double-semicolons: "goto error;;".
     Ben Pfaff
     pspp-dev mailing list

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: signature.asc
Description: Digital signature

reply via email to

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