[Bug 24258] Improve valgrind-cleanness

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24258





--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-12-03 08:58:57 PST ---
(In reply to comment #2)
> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=commitdiff;h=94dae71c582130aa053c953311b604b6d8f04e28
> which required me to look both at the commit message and the full source to see
> what was going on. Would having a VALGRIND_TESTS variable make it clearer?

It would make this diff clearer, but at the cost of adding a somewhat
pointless layer of abstraction.

Explanation: we have two kinds of test in this directory: compiled C,
and Python. All the compiled tests are in $(noinst_PROGRAMS); TESTS
contains those plus the Python test. When valgrinding, we don't want to
valgrind the Python test.

Perhaps a clearer structure, which doesn't add a strange layer of
abstraction, would be:

dist_noinst_SCRIPTS = all-errors-documented.py
noinst_PROGRAMS = ...

TESTS = \
    $(noinst_PROGRAMS) \
    $(dist_noinst_SCRIPTS)

# skip the scripts when valgrinding - we don't want to valgrind the
# interpreter
check-valgrind:
        $(MAKE) check-TESTS \
                dist_noinst_SCRIPTS="" \
                TESTS_ENVIRONMENT="$(VALGRIND_TESTS_ENVIRONMENT)"
        $(MAKE) -C dbus check-valgrind

Would you be happier with that?

> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=commitdiff;h=7c0af0be354edad11930d2d0e6de897ae7a57253
> If we want to have a test that checks whether valgrind likes our program,
> wouldn't it be better to have it fail if valgrind doesn't like us?

You'd think... but in practice:

* any false positives in libc, libdbus, dbus-glib cause the tests to fail,
  and the rest of the tests not to run

* memory leaks don't count as an error, so they don't cause the tests to
  fail, so you have to grep the output anyway

> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=commitdiff;h=63ceaa9286a5b5ba1cf30b9954601140b6b7893d
> looks good if my guess at what the ... syntax means is correct.

"..." means "zero or more stack frames"; it's very useful, but I didn't know
about it when I wrote the initial suppressions file.

> -- Have you made any changes that could/should be published to
> http://live.gnome.org/Valgrind?

It should probably be referenced there and/or copied into Johan's
suppressions file, yes. I'd prefer to get it merged first, so we can
just point into gitweb...

Johan's suppressions are too strict too :-P

> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=commitdiff;h=8f69846e0eacf758f35eb3d8d1f6034e243e6bb2
> -- I like these kinds of commits.

Me too :-)

> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=commitdiff;h=2fa0d932088ca4d8812bbf3f46f787d51ff94730
> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=commitdiff;h=2d11ca85e44f33f590b352559c3b8016d7673945
> -- Would it be possible to make our tests drop off the bus before exiting, or
> is this too much work?

Too much work for right now, certainly! MC jumps through hoops to get this
to work, but it took quite a bit of coding to get there.

You're not allowed to disconnect the shared session bus, so any test
that used tp_dbus_daemon_dup() or dbus_g_bus_get() or tp_get_bus() would
have these leaks anyway, unless we contrived some way to induce the session
bus to kick us off (which is liable to get us killed by libdbus assertion
failures).


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.