search

intel / dleyna-renderer

dleyna-renderer is a library for implementing services that allow clients to discover and manipulate Digital Media Renderers. An implementation of such a service for linux is also included
https://01.org/dleyna/
GNU Lesser General Public License v2.1
16 stars 19 forks source link

Valgrind reports memory corruption when Setting the Mute property #37

Closed markdryan closed 11 years ago

markdryan commented 11 years ago

To Reproduce:

  1. Start XBMC.
  2. Start dleyna-renderer with valgrind, e.g., libtool --mode=execute valgrind --num-callers=40 server/dleyna-renderer-service
  3. cd dleyna-renderer/test/dbus
  4. python
  5. from rendererconsole import *
  6. manager = Manager()
  7. ren = Renderer("/com/intel/dLeynaRenderer/server/0")
  8. ren.set_prop("Mute","org.mpris.MediaPlayer2.Player", not ren.get_prop("Mute"))

    Expected Result:

No errors in valgrind output

Actual Result:

==3297== Conditional jump or move depends on uninitialised value(s)
==3297==    at 0x533B56A: prv_rc_last_change_cb (device.c:1435)
==3297==    by 0x62E36E2: emit_notifications (gupnp-service-proxy.c:1683)
==3297==    by 0x5082AB4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3400.1)
==3297==    by 0x5082DE7: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3400.1)
==3297==    by 0x50831E1: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3400.1)
==3297==    by 0x4E355D0: dleyna_main_loop_start (main-loop.c:153)
==3297==    by 0x400EBA: main (daemon.c:93)

Analysis

Valgrind is correct about the line number but the error is confusing. It reports a problem executing

mpris_volume = (double)device_volume / (double)device->max_volume;

because if you set the Mute value, the event only contains a value for mute. It does not contain a value for Volume, and so device_volume is not initialised. I don't get an error when setting Volume.

Here's the event I get when I set the Mute property

<Event xmlns="urn:schemas-upnp-org:metadata-1-0/RCS/"><InstanceID val="0"><Mute Channel="Master" val="1"/></InstanceID></Event>

Note no Volume. So I think we just need to initialise the volume in the same way as Mute.

We could set it to either MAX_UINT or device->max_volume + 1.

markdryan commented 11 years ago

Fixed by PR #47