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

[Ben Pfaff]

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
<http://savannah.gnu.org/patch/download.php?file_id=10341>

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?func=detailitem&item_id=5209>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/