phonon static analysis

[Erik Hovland]

>> I ran my static analysis tool over phonon. It developed some
>> interesting patches.
>>
>> Anyhow, I have mailed those patches to Matthias (kretz at kde.org). Let's
>> hope it improves the phonon gstreamer experience a little.
>
> I'd love to see the patches that you, the lord of code fixing and c++
> quirks, calls "interesting"  :-)

Well, the strangest thing is the sentinel stuff in C ellipse functions. A
similar issue was found in Amarok 2.0 when I started scanning it. But it
is one of those annoyances of using any glib/gtk based library.

For example, if there is a function that uses an ellipse and you don't want to
use the last format string in the function, in C you might put null:
g_object_set(G_OBJECT(m_effectElement), "volume", volume, NULL);

Which in C is perfectly legal. NULL happens to have special type
status in GNU C.

But in C++, such type status does not exist. So GNU C++ types it to a
unsigned int (of 32-bits). On a 32-bit system, not a problem. On a
64-bit system,
a pointer is 64-bits. So now there is a type mismatch. And bad things
can happen.
The worse of which is that some noise wanders into those upper 32-bits and
that function call now treats that NULL like there is actually
something there. Oh noes!

The proper C++ way to make that call is:
g_object_set(G_OBJECT(m_effectElement), "volume", volume, (const char*) NULL);

I bet GCC C++ in the 4.4 series starts putting out warnings for this
issue. But currently
it hobbles along silently unless you have a static analysis tool that
knows how to notice
it.

The second thing is my favorite idiom to hate - the
QSharedDataPointer/QSharedData
usage. If you get a heap allocated pointer of something that derived
from QSharedData
and you don't store that pointer in a QSharedDataPointer (perhaps
because the function
signature can't tell you to use a QSharedDataPointer) then you will
leak that memory. Or,
better yet you will get strange segfaults if you try to sweep that
memory yourself. But that
is a minor nit. I just wish that the idiom did not have pitfalls that
can happen even after
the code has been compiled.

Otherwise, most of the defects were in the gstreamer code. But they did not
have bright shiny "ZOMG, fix me" written all over them. I expect only
a marginal improvement
once my patches get applied.

E

-- 
Erik Hovland
erik at hovland.org
http://hovland.org/