Bug 160956

Created attachment 24377 [details]
Mallinfo patch

Applied (slightly modified) patch on the trunk. Can you please test whether the version on the trunk is OK ? See also http://www.valgrind.org/downloads/repository.html for instructions on how to download and build the trunk.

I've created a test for mallinfo that checks sanity and basic features.
In fact, my original implementation does not passed it, so here's a patch on top of your commit.

Created attachment 24416 [details]
Test case

Created attachment 24417 [details]
Patch to pass the test

Thanks for contributing the new patch and regression test. Do you know of any formal standards that document the behavior of mallinfo() ? How can we know that the behavior tested by the regression test matches the behavior of all glibc versions supported by Valgrind ? And how about mallinfo() on AIX and FreeBSD ? AIX is supported since some time by Valgrind, and there is a project ongoing to port Valgrind to FreeBSD.

In theory mallinfo is defined in SVID2 and XPG4. However I don't have a copy of these and they are old standards. 4th edition of SVID (available from SCO: http://www.sco.com/developers/devspecs/vol1a.pdf) deprecates mallinfo() and doesn't document struct mallinfo. XPG4 evolved into SUS which doesn't have mallinfo.

Moreover, glibc implementation never actually followed the standard. Definition of struct mallinfo here http://docsrv.sco.com:507/en/man/html.S/malloc.S.html (which I believe is what was in SVID2, because HP and SunOS give exactly the same) is quite different from what glibc has. So the only definition for mallinfo on Linux is malloc.h/glibc sources (also note that the link you added to comments - http://www.gnu.org/software/libtool/manual/libc/Statistics-of-Malloc.html - is somewhat outdated; in particular, fields marked as "not used" are currently used).

Most of my regtest checks should work with both standard and glibc implementations, though there are glibc specific bits. I think this makes sense since most programs using mallinfo on Linux will use glibc specific behaviour. According to glibc cvs the first version that should pass my regtest was checked in 1996, so this should not be a problem.

There is no mallinfo on FreeBSD, so no problem here.

I don't have access to any AIX systems, but I suspect that AIX also gave it's own meaning to mallinfo fields. For AIX we can either return zeroes as it was before or someone with access to AIX can provide a version of implementation and regtest that matches AIX behaviour. This means making mallinfo OS specific.

A slightly modified version of the last two patches is present in revision 7902. Can you please review the changes and close this issue if you agree with the committed implementation ?

Looks good to me. Actually, am I missing something or it only differs in comments?
Anyway, as long as it passes my test it's fine with me. Thanks!

btw, I suggest to move test from none to memcheck. none doesn't redirect malloc and friends, so the test actually tests host libc's implementation of mallinfo, not valgrind's implementation (this can be seen in test's output).

Next to adding comments, I also added a command-line option -q to the regression test and I added a few lines of code to free all allocated memory, such that memcheck does not complain about memory leaks. The mallinfo regression test has been moved from the directory none/tests to memcheck/tests by this time.

Ah yes, I noticed improvements to the test, but I was looking for changes to implementation. The -q option is great, actually without it the test may fail if stdio implementation allocates buffers with malloc, but prints are useful for debugging, so it's good to have it an option.
Thanks again.

*** Bug 161251 has been marked as a duplicate of this bug. ***

Mistake; #161251 is NOT a duplicate of this bug.  Ignore Comment #12.