[Goodies-dev] [Bug 4690] xfce4-sensors-plugin crashed with SIGSEGV in free() at every reboot

[bugzilla-daemon at xfce.org]

http://bugzilla.xfce.org/show_bug.cgi?id=4690





--- Comment #9 from Dave Witbrodt <dawitbro at sbcglobal.net>  2009-03-24 01:07:36 UTC ---
(In reply to comment #8)
> well first of all, you could have tried to simply run the plain binary program
> xfce4-sensors which -- who would expect -- links to the very same library with
> al lthe hddtemp stuff inside. This executable could be seen in your
> installation logs etc.

You overlooked a couple of factors in your hurry to be sarcastic:

1)  When first investigating the crash, it was possible that parameters were
being passed to the plugin that were causing it to die.  Debugging it by
directly running the plugin binary would never have caught such a problem.  (Of
course, in hindsight, we _now_ know that your approach would have worked.)

2)  I have never looked at XFCE code before, much less tried to debug a
crashing plugin using 'gdb'.  I know you didn't have time to read that whole
thread on the Debian BTS, but if you _had_, you would have seen that I asked
the Debian maintainer for guidance on how to proceed... and was directed to
this information on the xfce.org website:

http://wiki.xfce.org/howto/panel_plugin_debug

"Who would expect" that the xfce.org website would provide debugging advice
that would cause XFCE developers to mock people who use it to try to provide
fixes for their broken code?  If you think those instructions are incorrect,
you might consider going there and making the necessary changes.


> Anyway, you're right about what breaks it that uninitialized variables
> couldn't be free properly with *new* versions of gcc; I didn't have any
> problems earlier.

Well, I wouldn't fault 'gcc'.  I found the problem by comparing the working
version to the new version, with the addition of the new code involving
"checktext".  By declaring the variable at the top of the function, and freeing
it near the end, the developer who introduced these changes made it possible to
easily overlook that fact that not all code paths initialize the variable.


> Sorry to tell, moving variable declaration inside C code is *not* a solution
> because of 1) variables should be declared at the beginning

Well, I guess someone needs to explain that to my compiler... since the changes
I made work fine right now.

You cannot seriously be arguing that variables _have_ to be declared at the
beginning of a function, since the C language allows any "compound statement"
(a block of code enclosed in braces) to begin with an (optional) declaration
list.  The GNU C Reference puts it this way:  "You can declare variables inside
a block; such variables are local to that block."

http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Blocks

A more official language reference puts it more tersely:

http://techpubs.sgi.com/library/dynaweb_docs/0650/SGI_Developer/books/CLanguageRef/sgi_html/ch08.html#Z86421


> 2) it would not resolve your issues when any of the mentioned else if
> branches would be entered.

I used the approach of moving both the declarations of "checktext" and the
corresponding call of g_free(checktext) into the blocks where "checktext" was
actually being used.  If you look carefully at the if-else structure in
get_hddtemp_value(), you will see that only two blocks even need "checktext" at
all, so there is no point for "checktext" to exist anywhere else in the code
_except_ in those two blocks.  It was the fact that "checktext" was freed
_outside_ of those blocks that introduced the bug.

Not being familiar with g_free(), I was not sure if merely initializing it to
NULL would be a correct solution.  OTOH, I _was_ sure that limiting the scope
of "checktext" to only those blocks where it was needed, and freeing it in
those blocks, was guaranteed to be a workable solution.  (Assuming no other
bugs were present besides this one.)


> SVN will soon contain initialization to NULL which should work. Thanks to the
> guy that pointed that out and thanks to you for reporting and all the efforts
> you investigated when submitting the bugs at Debian.

Well, thank you for applying a fix for the problem... and sorry to have
distressed anyone because of my lack of familiarity with debugging XFCE
plugins, with debugging in general, and with my lack of familiarity with the
behavior of library functions such as g_free() so that I proposed the sort of
scope usage that would have made this bug impossible in the first place!  In
the future, I'll make sure I read the XFCE wiki first so that I don't make a
fool....  Oh, never mind!  ;)


Sincere gratitude for XFCE and its development team!
Dave W.

-- 
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.