[Top][All Lists]

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

PSPP-BUG: [patch #5209] Proposed casefile changes

From: Ben Pfaff
Subject: PSPP-BUG: [patch #5209] Proposed casefile changes
Date: Wed, 12 Jul 2006 22:37:42 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20060406 Firefox/ (Debian-1.5.dfsg+

Update of patch #5209 (project pspp):

             Assigned to:                     blp => jmd                    


Follow-up Comment #5:

The second patch you posted is closer to what I had in mind, but I
still think there is room for improvement.

Some comments:

* Nitpick: I find "klass" annoying to look at.  I know that certain
  projects use that spelling a lot, but "class" works just fine in C.
  If we want to convert PSPP to C++ then we have bigger problem in my
  opinion.  So let's just use "class".

* Many of the asserts are redundant with what is going to happen
  anyway a few lines later, and they make the code harder to read.

* The casting-via-macros-that-invoke-functions-that-assert trick is
  perhaps a little safer, but I don't think it's enough to be really
  worth the extra obfuscation.  After all, the functions that call
  these have generally been invoked through a function pointer via the
  very same structure that they're checking.  The kind of problem that
  would cause an assertion failure here but not a bad jump is pretty
  unlikely in my opinion.

* We can initialize the classes at compile time and make them const,
  and there's not really a good reason to embed them with a separate
  "initialised" member in an outer structure if we do that.

This is all kind of abstract, and I really hope I don't sound like
some kind of a jerk saying it.  Anyhow, I implemented these changes in
casefile.c and fastfile.c and attached a patch illustrating them.
flexfile.c would want similar changes but I only did the minimum
necessary there.


Additional Item Attachment:

File name: alternative.patch              Size:108 KB
proposed change


Reply to this item at:


  Message sent via/by Savannah

reply via email to

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