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 InterfaceSeverity: 7 - Major
Status: FixedAssigned to: John Darrington <jmd>
Open/Closed: ClosedRelease: 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 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 File(s):
   
   
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
  • -unavailable- added by blp (Posted a comment)
  • -unavailable- added by jmd (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 14 Sep 2007 01:46:08 PM UTCblpStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Fri 14 Sep 2007 05:23:21 AM UTCblpAttached File-=>Added clone-preserve-case-indexes.patch, #13938
    Thu 13 Sep 2007 07:26:04 AM UTCjmdAttached File-=>Added patch, #13933
    Thu 13 Sep 2007 02:54:27 AM UTCjmdReleaseNone=>Before 0.6.0

    Back to the top


    Powered by Savane 3.1-cleanup1