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

 
 

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

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

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( 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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.

 

(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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    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.6