| | [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #2005
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 4 - High, e.g. key functionality not working |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | Core Library |
|---|
| Summary: | Fix for free(lineStr) in Fl_Text_Display.cxx |
|---|
| Version: | 1.3-current |
|---|
| Created By: | markcw |
|---|
| Assigned To: | fabien |
|---|
| Fix Version: | 1.3-current (SVN: v6387) |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | markcw
17:03 Jul 15, 2008 |
| A check added to Fl_Text_Display.cxx not by me but by Sebastian Hollington. It is an if() check to avoid freeing lineStr if it's not there, which was causing random crashes. It is tested in Ubuntu but probably applies to all OS.
The fix is shown below and the modified Fl_Text_Display.cxx is attached.
// Line 1480 in Fl_Text_Display::draw_vline
stdCharWidth = TMPFONTWIDTH; //mFontStruct->max_bounds.width;
if ( stdCharWidth <= 0 ) {
Fl::error("Fl_Text_Display::draw_vline(): bad font measurement");
if(lineStr) free((void *)lineStr); //Seb was here - added if() check
return;
} | |
| |
| #2 | matt
08:32 Jul 18, 2008 |
| Should be applied to 1.3 because 1.1.9 is final. | |
| |
| #3 | fabien
06:18 Sep 02, 2008 |
| no doubt that the pointer should be checked here,
but what intringues me is : do we address correctly the problem only with this, why and when a bad fond measurement could apply ? | |
| |
| #4 | AlbrechtS
08:08 Sep 02, 2008 |
| I agree that the check is correct/useful, because lineStr could be set to NULL some line above this.
However, I can't see, how this if-condition could ever be true. TMPFONTWIDTH is a constant that is defined to be 6. Thus, this condition can never be true, and the compiler should optimize it away.
If the OP experienced crashes, then maybe because he changed more in the original code (maybe he un-commented this "//mFontStruct->max_bounds.width;" instead of using the constant TMPFONTWIDTH ?). There are more occurrences of TMPFONTWIDTH instead of mFontStruct->..., together with a FIXME comment in line 1757.
Note also: There's another diff in line 386ff:
@@ -386,6 +384,7 @@
for (i = 0, mMaxsize = fl_height(textfont(), textsize()); i < mNStyles; i++)
mMaxsize = max(mMaxsize, fl_height(mStyleTable[i].font, mStyleTable[i].size));
+ mMaxsize+=2; //simon was here
... | |
| |
| #5 | markcw
12:52 Sep 03, 2008 |
| Hi fabien, the way I see it there is no need to check stdCharWidth any more because the original line read:
stdCharWidth = mFontStruct->max_bounds.width;
But note that neither Seb or simon changes anything to do with mFontStruct or TMPFONTWIDTH, this was already there in the original 1.1.9 file.
It appears that the mFontStruct was causing problems and replaced by TMPFONTWIDTH but according to the person who fixed it (CET?) it is only a temp fix to avoid crashes. I don't know what this mFontStruct is so can't say any more. | |
| |
| #6 | markcw
12:56 Sep 03, 2008 |
| Ah, mfontstruct is an XFontStruct. | |
| |
| #7 | markcw
13:04 Sep 03, 2008 |
| Oh you're right, the check should never happen if TMPFONTWIDTH is always 6. I don't know then, I just post these on behalf of Seb as he had a problem signing up to the forum. I'll point it out to him. | |
| |
| #8 | fabien
16:37 Oct 05, 2008 |
| Fixed in Subversion repository.
I added a check before the free() call because if one day the 'temporary fix' is removed, we won't crash here.
But it will make no difference though, until the TMPFONTWIDTH workaround disappears when X11 problems (bugs?) will be solved. | |
[ Return to Bugs & Features ]
|
| |
