bugPSPP - Bugs: bug #21061, Inserting a variable confuses the...

 
 

bug #21061: Inserting a variable confuses the dictionary on save.

Submitter:  John Darrington <jmd>
Submitted:  Thu 13 Sep 2007 02:54:06 AM UTC
   
 
Category:  Graphical User Interface Severity:  7 - Major
Status:  Fixed Assigned to:  jmd
Open/Closed:  Closed Release:  Before 0.6.0
Effort:  0.00
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 14 Sep 2007 01:46:08 PM UTC, comment #6: 


>Your patch seems to work fine, and it's much simpler than mine.


Great, I checked it in.

I noticed that you'd misspelled your name in two ChangeLog entries, so I fixed that also 8-)

Ben Pfaff <blp>
Group administrator
Fri 14 Sep 2007 07:56:57 AM UTC, comment #5: 

Your patch seems to work fine, and it's much simpler than mine.

John Darrington <jmd>
Group administrator
Fri 14 Sep 2007 05:24:22 AM UTC, comment #4: 


>A patch set that I produced for bug #6117 was supposed to do that.


Excuse me, that should be patch #6117, not bug.

Ben Pfaff <blp>
Group administrator
Fri 14 Sep 2007 05:23:21 AM UTC, comment #3: 


>The problem is that first it calls dict_clone. Unfortunately
>dict_clone doesn't preserve case indices. Consequently, the
>case_map is all wrong.


A patch set that I produced for bug #6117 was supposed to do that.  Somehow it never got checked in, even though the rest of the patch set seems to have been.  I'm attaching the missing patch here.

I didn't analyze why your patch causes assertion failures in some tests.  Mine doesn't, and it still fixes the problem.  What do you think of it?

BTW, I reviewed our current usages of dict_clone and it looks like it isn't necessary to insert any dict_compact_values calls to comply with the suggestion in the comment that this patch adds.

(file #13938)

Ben Pfaff <blp>
Group administrator
Thu 13 Sep 2007 07:26:04 AM UTC, comment #2: 

I had a look at this.  parse_write_command ( in get.c ) recognises this as a potential problem, and correctly tries to handle it by setting up a case_map and translator.

The problem is that first it calls dict_clone.  Unfortunately dict_clone doesn't preserve case indices. Consequently, the case_map is all wrong.

I hacked up this patch, which changes dict_clone to preserve the case_indices.  It fixes the problem, but causes 4 other tests to assert in dict_delete_var

Perhaps there's a better solution.

(file #13933)

John Darrington <jmd>
Group administrator
Thu 13 Sep 2007 05:31:58 AM UTC, comment #1: 

On initial investigation this is a bit puzzling, as LIST and other data handling procedures get it right.  There must be something special about SAVE that I am not seeing.

Ben Pfaff <blp>
Group administrator
Thu 13 Sep 2007 02:54:06 AM UTC, original submission:  

This bug can be reproduced as follows:

1. Open the GUI.

2. Open a system file containing at least one variable and one case.

3. Insert a new variable at the start of the dictionary.

4. Enter data for that variable.

5. Save the system file under a new name.


The saved system file will be saved with data corresponding to the wrong variables.

John Darrington <jmd>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #13933:  patch added by jmd (5KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by blp (Posted a comment)
  • -email is unavailable- added by jmd (Submitted the item)
  •  

    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.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-09-14 blp StatusNone Fixed
        Open/ClosedOpen Closed
    2007-09-14 blp Attached File- Added clone-preserve-case-indexes.patch, #13938
    2007-09-13 jmd Attached File- Added patch, #13933
    2007-09-13 jmd ReleaseNone Before 0.6.0

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code