View Issue Details [ Jump to Notes ]

[ Print ]

Project

Category

View Status

Date Submitted

Last Update

0007141

KWWidgets

public

2008-06-02 16:43

2008-06-03 09:29

Reporter

kentwilliams

Assigned To

Yumin Yuan

Priority

normal

Severity

minor

Reproducibility

always

Status

resolved

Resolution

fixed

Platform

OS Version

Summary

0007141: vtkKWLoadSaveDialog initially lists '1' as the file type in the 'Files of type:' combo box.

Description

vtkKWLoadSaveDialog initially lists '1' as the file type in the 'Files of type:' combo box.

Additional Information

I tracked this down and provide a patch.

The problem is this: first, a call to vtkKWComboBox::GetValueFromIndex() is made. This calls this->Script to get the value out of the TK widget.

Then a call is made to vtkKWComboBox::SetValue(), which calls vtkKWWidget::IsAlive(), which calls vtkKWWidget::IsAlive(), which calls vtkKWApplication::EvaluateBooleanExpression() which calls vtkKWTkUtilities::EvaluateString() to get the value.

But ... deep breath ... EvaluateString is the same function that vtkKWComboBox::GetValueFromIndex() calls to get the Nth string from the Combo box widget. And EvaluateString returns a static char * value that is overwritten every time it is called.

The solution is to save the string returned from GetValueFromIndex, before calling SetValue.

Tags

No tags attached.

Attached Files

vtkKWFileBrowserDialog.cxx.patch [^] (1,175 bytes) 2008-06-02 16:43 [Show Content] [Hide Content]

Index: vtkKWFileBrowserDialog.cxx
===================================================================
--- vtkKWFileBrowserDialog.cxx	(revision 94)
+++ vtkKWFileBrowserDialog.cxx	(working copy)
@@ -563,10 +563,9 @@
 
     if (this->FileTypesBox->GetNumberOfValues() > 0)
       {
-      this->FileTypesBox->SetValue(
-        this->FileTypesBox->GetValueFromIndex(0));
-      this->FileTypeChangedCallback(
-        this->FileTypesBox->GetValueFromIndex(0));
+      std::string val(this->FileTypesBox->GetValueFromIndex(0));
+      this->FileTypesBox->SetValue(val.c_str());
+      this->FileTypeChangedCallback(val.c_str());
       }
 
     if (this->InitialFileName && *this->InitialFileName)
@@ -671,10 +670,9 @@
     this->PopulateFileTypes();
     if (this->FileTypesBox->GetNumberOfValues() > 0)
       {
-      this->FileTypesBox->SetValue(
-        this->FileTypesBox->GetValueFromIndex(0));
-      this->FileTypeChangedCallback(
-        this->FileTypesBox->GetValueFromIndex(0));
+      std::string val(this->FileTypesBox->GetValueFromIndex(0));
+      this->FileTypesBox->SetValue(val.c_str());
+      this->FileTypeChangedCallback(val.c_str());
       }
     }
 }

Relationships

Notes
(0012210) Yumin Yuan (developer) 2008-06-03 09:24	CVS commit BUG: Try the fix again. (silly me for last reverse). The returned value (const char) from vtkKWComboBox::GetValueFromIndex() is only pointing to a temporary buffer, and it should be copied locally for later use. FYI, Patrick Emond reported this problem a couple weeks ago, and I committed a fix, but later I reversed that fix because the File Browser was broken due to other changes in vtksys, and I never put that fix back in after figuring out why the file browser was broken. On Mon, Jun 2, 2008 at 4:58 PM, kent williams <nkwmailinglists@gmail.com> wrote: > I logged this in Mantis -- http://www.itk.org/Bug/view.php?id=7141 [^] -- > I provide a patch for this specific problem, but I worry that there > are other potential issues elsewhere in KWWidgets. > ... There are some other methods in KWW classes, just like vtkKWComboBox::GetValueFromIndex() , the returned value is a pointer to a temporary buffer, which should be copied immediately* to your own storage. If you grep temporary buffer in KWW header files, you will see most (if not all) of these methods. Apparently, the doc (comments) for vtkKWComboBox::GetValueFromIndex() did not say that :-(, and I added comments to show this feature.

(0012211) Yumin Yuan (developer) 2008-06-03 09:28	More comments from Sebastien: On Tue, Jun 3, 2008 at 8:56 AM, Sebastien BARRE <sebastien.barre@kitware.com> wrote: > At 6/2/2008 10:58 PM, kent williams wrote: > >> What the bug comes down to is this: If you make any of the myriad >> calls that end up using ad-hoc TCL scripts to pull values out of TK >> widgets, you have to save the result before any calling any other >> method that ends up using a TCL script. > > Yes, it's a well known issue, and we have tried to document it for every > method that returns a pointer to that specific buffer (i.e. it mostly > concerns methods returning char*). If it bites you in a specific class, let > us know and we will add more documentation. >

Issue History
Date Modified	Username	Field	Change
2008-06-02 16:43	kentwilliams	New Issue
2008-06-02 16:43	kentwilliams	File Added: vtkKWFileBrowserDialog.cxx.patch
2008-06-03 08:51	Sebastien Barre	Status	new => assigned
2008-06-03 08:51	Sebastien Barre	Assigned To	=> Yumin Yuan
2008-06-03 09:24	Yumin Yuan	Note Added: 0012210
2008-06-03 09:28	Yumin Yuan	Note Added: 0012211
2008-06-03 09:29	Yumin Yuan	Status	assigned => resolved
2008-06-03 09:29	Yumin Yuan	Resolution	open => fixed