Hi Jeff,
I'd be okay with this as long as there would be a config option to revert
back to using malloc rather than calloc.
Howard
On 10/3/14 2:54 PM, Jeff Squyres (jsquyres) wrote:
WHAT: change the malloc() to calloc() in opal_obj_new() (perhaps only in debug
builds?)
WHY: Drastically reduces valgrind output
WHERE: see
https://github.com/open-mpi/ompi/blob/master/opal/class/opal_object.h#L462-L467
TIMEOUT: teleconf, Tue, Oct 14 (there's no rush)
MORE DETAIL:
I was debugging some code today and came across a bunch of places where we
write structs down various IPC mechanisms, and the structs contain holes. In
most places, the performance doesn't matter / the readability of struct members
is more important, so we haven't re-ordered the structs to remove holes. But
consequently, those holes end up uninitialized, and therefore memcpy()ing or
write()ing instances of these structs causes valgrind to emit warnings.
The patch below eliminates most (all?) of these valgrind warnings -- in debug
builds, it changes the malloc() inside OBJ_NEW to a calloc().
Upon a little more thought, however, I wonder if we use OBJ_NEW in any fast
code paths (other than in bulk, such as when we need to grow a free list).
Specifically: would it be terrible to *always* calloc -- not just for debug
builds?
-----
diff --git a/opal/class/opal_object.h b/opal/class/opal_object.h
index 7012bac..585f13e 100644
--- a/opal/class/opal_object.h
+++ b/opal/class/opal_object.h
@@ -464,7 +464,11 @@ static inline opal_object_t *opal_obj_new(opal_class_t * cl
opal_object_t *object;
assert(cls->cls_sizeof >= sizeof(opal_object_t));
+#if OPAL_ENABLE_DEBUG
+ object = (opal_object_t *) calloc(1, cls->cls_sizeof);
+#else
object = (opal_object_t *) malloc(cls->cls_sizeof);
+#endif
if (0 == cls->cls_initialized) {
opal_class_initialize(cls);
}
-----