patchPSPP - Patches: patch #5567, formatted data input update and...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5567: formatted data input update and regression suite

Submitter:  Ben Pfaff <blp>
Submitted:  Sun 19 Nov 2006 05:32:40 PM UTC
   
 
Category:  None Item Group:  None
Status:  Done Assigned to:  blp
Open/Closed:  Closed

Wed 22 Nov 2006 04:20:11 AM UTC, comment #2: 


>There are a number of places where you have +char tmp[6];+ and
>later you are indexing into tmp with some integer expression. I
>wonder if it would be safer to use +struct string+ in these places
>instead?


OK, I've done this now.  I reflexively avoid things like this because they seem slower, but it is probably not a common enough path to matter (esp. in parse_Z).

>There are a number of instances of isxdigit, and other instances
>of isdigit/isspace/isalpha. Are you sure that locale/non-locale
>dependent versions are being used where appropriate?


Good point, these should all be c_isxdigit etc.  I fixed them.

>I still think that it may be a good idea (at least eventually) to
>allow locale dependent versions of data input.


Definitely.  I'm just not working on that right now.

>Di Okt 21 18:49:56 CST 2006
>Mi Okt 22 17:59:36 CST 2006


Are month names in German more than one word then?  Multi-word names are going to be a little more work.

Ben Pfaff <blp>
Group administrator
Tue 21 Nov 2006 10:53:30 AM UTC, comment #1: 

Only a few points:

  1. There are a number of places where you have +char tmp[6];+ and later you are indexing into tmp with some integer expression.  I wonder if it would be safer to use +struct string+ in these places instead?


  1. There are a number of instances of isxdigit, and other instances of isdigit/isspace/isalpha.  Are you sure that locale/non-locale dependent versions are being used where appropriate?


  1. I still think that it may be a good idea (at least eventually) to  allow locale dependent versions of data input.  For example, users in a German locale might well have a large text file of the form:


Di Okt 21 18:49:56 CST 2006
Mi Okt 22 17:59:36 CST 2006

etc
which they'd want to parse into dates in PSPP.


John Darrington <jmd>
Group administrator
Sun 19 Nov 2006 05:32:40 PM UTC, original submission:  

This patch rewrites the formatted data input routines to conform closely to what my tests show SPSS doing.  Perhaps more importantly, it adds numerous regression tests to make sure that PSPP behavior won't change in this area without us noticing.

Comments?

Ben Pfaff <blp>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #11299:  data-in.patch added by blp (175KiB - text/x-patch - patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

 

Follow 4 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2006-11-25 blp StatusReady For Test/Review Done
    Open/ClosedOpen Closed
2006-11-21 jmd Assigned tojmd blp
2006-11-19 blp Attached File- Added data-in.patch, #11299

Back to the top

Powered by Savane 3.13-bb6a.
Corresponding source code