Lin:

> I'm curious why debug enabled libmetacity can't cause a crash and why 
> crash doesn't happen on SPARC.
> Your fix may work well but it's not explanation of the reason to me.

Leaving the height undefined will not have consistent behavior between
builds, since the undefined memory value could be different from build
to build, and between x86 and Sparc.  So the behavior of this sort of
memory problem would be expected to vary.

Using vermillion build 90, the undefined value was very large which
was causing a crash in replicate_rows in metacity/src/ui/theme.c
The call to gdk_pixbuf_new was returning NULL because the large
height value was causing it be unable to allocate memory.  The
NULL value was passed into a function which failed due to an assert.

I found that I could not get dbx to stop on the SEGV when I simply
ran "dbx /usr/bin/gnome-appearance-properties" and typed "run".  Note
that the crash happened in the thumbnailer, which is run as a forked
subprocess.  So I added a "sleep (15)" to the code, just after the
fork in gnome-control-center/capplets/common/theme-thumbnail.c in
the function theme_thumbnail_factory_init.  Then I attached
the dbx debugger to the child process while it was sleeping.
After going through this hassle, the debugger was able to show
me the SEGV and it was a lot easier to track down the problem.

Brian


> Brian Cameron wrote:
>>
>> I found that the appearance bug reappeared in vermillion-devel 90, and
>> disabling mediaLib had no affect on things.  So my previous patch
>> to workaround the problem just hid the issue in vermillion-devel 89.
>>
>> Doing further research, it turns out that the problem was caused
>> by the metacity-08-trusted-extensions.diff file not setting
>> fgeom->height when TSOL isn't being used.
>>
>> Since the affected patch is huge, and the change is small, I'll just
>> explain my change rather than showing a diff file.
>>
>> I changed this hunk of the patch:
>>
>> ----
>>
>> @@ -621,8 +626,16 @@
>>      fgeom->top_height + fgeom->bottom_height;
>>
>>    fgeom->width = width;
>> +#ifdef BUILD_TX_CODE
>> +#ifdef HAVE_XTSOL
>> +  if (tsol_is_available ())
>> +    fgeom->height = height + fgeom->top_height; /*Trusted Frame 
>> Layout Modifica
>> tion TFLM*/
>> +  else
>>    fgeom->height = height;
>> -
>> +#else
>> +  fgeom->height = height;
>> +#endif
>> +#endif
>>    fgeom->top_titlebar_edge = layout->title_border.top;
>>    fgeom->bottom_titlebar_edge = layout->title_border.bottom;
>>    fgeom->left_titlebar_edge = layout->left_titlebar_edge;
>>
>> ----
>>
>> Note that fgeom->height never gets set if BUILD_TX_CODE is not TRUE.
>>
>> So I changed it to the following code, which always sets fgeom->height
>> to height, and then resets it to the appropriate TSOL value if the
>> #defines and if-test pass.  I think this code is a bit more simple
>> than dealing with so many embedded #else statements.
>>
>> ----
>>
>> @@ -622,7 +627,12 @@ meta_frame_layout_calc_geometry (const M
>>
>>    fgeom->width = width;
>>    fgeom->height = height;
>> -
>> +#ifdef BUILD_TX_CODE
>> +#ifdef HAVE_XTSOL
>> +  if (tsol_is_available ())
>> +    fgeom->height = height + fgeom->top_height; /*Trusted Frame 
>> Layout Modifica
>> tion TFLM*/
>> +#endif
>> +#endif
>>    fgeom->top_titlebar_edge = layout->title_border.top;
>>    fgeom->bottom_titlebar_edge = layout->title_border.bottom;
>>    fgeom->left_titlebar_edge = layout->left_titlebar_edge;
>>
>>
>>
>> ---
>>
>> Brian
> 


Reply via email to