Properly handle realloc() failures

[bryonglodencissp]

[hwloc/topology-linux.c:1787]: (error) Common realloc mistake: 'maps' nulled but not freed upon failure

Passing one pointer into realloc() and assigning the result directly into that same pointer variable can cause a memory leak if the reallocation fails, because the original allocation will still exist. The correct way to do this is to use a temporary pointer variable.

Credit 1: http://stackoverflow.com/a/11548901
Credit 2: RRZE-HPC/likwid#44

Found by https://github.com/bryongloden/cppcheck

[bgoglin]

Thanks, I'll apply this. That said, there are many places where we don't check the malloc return value anyway...

Does your cppcheck tool find any other issue in hwloc?

[bryonglodencissp]

Greetings @bgoglin. My tool has found 14 other instances of the same error...

[hwloc/topology-linux.c:2450]: (error) Common realloc mistake: 'ret' nulled but not freed upon failure
[hwloc/topology-linux.c:3679]: (error) Common realloc mistake: 'Lprocs' nulled but not freed upon failure
[hwloc/topology-solaris.c:526]: (error) Common realloc mistake: 'Pproc' nulled but not freed upon failure
[hwloc/topology-solaris.c:538]: (error) Common realloc mistake: 'Lproc' nulled but not freed upon failure
[hwloc/topology-solaris.c:587]: (error) Common realloc mistake: 'Lpkg' nulled but not freed upon failure
[hwloc/topology-solaris.c:633]: (error) Common realloc mistake: 'Lcore' nulled but not freed upon failure
[hwloc/topology-synthetic.c:995]: (error) Common realloc mistake: 'loops' nulled but not freed upon failure
[hwloc/topology-windows.c:756]: (error) Common realloc mistake: 'procInfo' nulled but not freed upon failure
[hwloc/topology-windows.c:875]: (error) Common realloc mistake: 'procInfoTotal' nulled but not freed upon failure
[hwloc/topology-xml-nolibxml.c:353]: (error) Common realloc mistake: 'buffer' nulled but not freed upon failure
[hwloc/topology-xml-nolibxml.c:687]: (error) Common realloc mistake: 'buffer' nulled but not freed upon failure
[hwloc/topology-xml-nolibxml.c:779]: (error) Common realloc mistake: 'buffer' nulled but not freed upon failure
[hwloc/topology.c:290]: (error) Common realloc mistake: 'infos' nulled but not freed upon failure
[hwloc/topology.c:320]: (error) Common realloc mistake: 'dst_infos' nulled but not freed upon failure

I believe it is a good idea to try fixing one before I trying anymore of them. This way, if there was a problem with collaboration, at least I wouldn't sacrifice too much time. Plus, I believe that sometimes people download the tool for themselves and use it to fix error themselves. Unfortunately, one of the obstacles on my end is that I'm unable to test my code changes. For example, when I changed this one file, topology-linux.c, and I ran the build process:

./autogen.sh
./configure
make
make install
lstopo

my system, an iMac running OS X El Capitan, just didn't compile the file. I'm not sure I know enough to know why it didn't compile, I just know that when I introduced a programming error into the file, and then rerun the previous commands, I wasn't presented with a build error (which is what I was expecting). I presume it didn't compile this "linux" file because I'm running a different OS.

All that being said. I'm happy to dive in and start fixing some of these, but I believe I need someone else to review my code changes (for my sanity and yours)!!!

For what it's worth, this tool also found this:

[hwloc/topology-solaris.c:431]: (error) Memory leak: glob_lgrps

I haven't look at the file yet, but these are usually easy to fix. I just wanted to get this comment out while I still had your attention 👍

[bgoglin]

@bryongloden I already have patches in my local tree for most realloc() calls including that one in nolibxml.c, I'll push everything to master and v1.11 tomorrow after more testing.

[bryonglodencissp]

Bonjour @bgoglin! Sorry I don't speak French, so bear with me through my English 👍

I was interested in testing the idea in your comment from #204. Namely, updating a branch via the GitHub GUI. Now that I know that works, I'll stop patching realloc() errors in the hwloc/hwloc directory 👍

Also, when you asked earlier if my tool found any more errors in hwloc, did you mean specifically in the hwloc/hwloc directory? Because that is/was the way I interpreted your question.

I've run the tool now across the entire hwloc repository and I've discovered errors/bugs inside hwloc/netloc, hwloc/tests, and hwloc/utils.

If you ping me tomorrow after you push your local edits, I'll sync up with master, rerun my tool, and open a new issue that includes a full list of all the errors I see. I believe this is the most efficient way for me to report the bugs to you (otherwise just download and build cppcheck yourself... and I'll go away) 👍

[bgoglin]

I pushed all realloc() fixes.
Some realloc() remain unfixed in the netloc lib and utils but this code is going to be deeply revamped, so fixing realloc() there is useless right now.