[Buildroot] [PATCH] valgrind: allow selecting which tools to install

On Wednesday 14 March 2012 17:50:59 Thomas De Schampheleire wrote:

The full valgrind installation takes more than 20 MB, while one typically does
not use all of its tools. This patch adds extra config options to select which
tools to install.

Signed-off-by: Thomas De Schampheleire <redacted>

 Strange that this patch didn't get any feedback before...  I guess we
all trust Thomas enough to make good patches :-)

 I'm a bit at loss as to what to do now.  I believe this patch is useful
and good enough to go in as it is.  So:
Acked-by: Arnout Vandecappelle (Essensium/Mind) <redacted>

 On the other hand, I do have some ideas how to make it even better.
But if I write these down, I'm afraid it will slow down the inclusion
of a useful and well-implemented feature...  Well, I'll take the risk:

[snip]

+config BR2_PACKAGE_VALGRIND_MEMCHECK
+	bool "Memcheck: a memory error detector"
+	default y
+	help
+	  This option allows to install the Memcheck tool
+
+config BR2_PACKAGE_VALGRIND_CACHEGRIND
+	bool "Cachegrind: a cache and branch-prediction profiler"
+	default y
+	help
+	  This option allows to install the Cachegrind tool

 Even though this breaks existing configurations, I believe that all
tools (with the possible exception of memcheck) should default to
no.  For existing configurations, that's what most people will want
anyway.  And for new configurations, I think it's usually just one,
maybe two tools that are needed, so it's a lot of work to disable
all of them.

[snip]

+config BR2_PACKAGE_VALGRIND_PTRCHECK
+	bool "Ptrcheck: an experimental head, stack and global array overrun detector"
+	default y
+	help
+	  This option allows to install the Ptrcheck tool

 At the very least, the experimental ones should default to no.

[snip]

+config BR2_PACKAGE_VALGRIND_NULGRIND
+	bool "Nulgrind: the minimal Valgrind tool"
+	default y
+	help
+	  This option allows to install the Nulgrind tool

 And this one definitely should default to no.

[snip]

+ifeq ($(BR2_PACKAGE_VALGRIND_MEMCHECK),)
+define VALGRIND_REMOVE_MEMCHECK
+	rm -f $(TARGET_DIR)/usr/lib/valgrind/*memcheck*
+endef
+
+VALGRIND_POST_INSTALL_TARGET_HOOKS += VALGRIND_REMOVE_MEMCHECK
+endif

 It's probably just personal preference, but I like to see:

define VALGRIND_REMOVE_MEMCHECK
	rm -f $(TARGET_DIR)/usr/lib/valgrind/*memcheck*
endef

VALGRIND_POST_INSTALL_TARGET_HOOKS += $(if $(BR2_PACKAGE_VALGRIND_MEMCHECK),,VALGRIND_REMOVE_MEMCHECK)

(although the double comma isn't making things very clear either...)


 It would actually be better to not build the unneeded tools at all, by 
setting
VALGRIND_MAKE_OPT = TOOLS="..." EXP_TOOLS="..."

 Unfortunately, Makefile.autotools.in doesn't use PKG_MAKE_OPT for the 
install targets (which is probably a bug...), so that won't work.

[snip]

 Regards,
 Arnout

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F