Clean up memory management in pmcstat

[stevek]

Summary

Calls to malloc need to test the returned pointer to ensure it is non-NULL.
Ensure all allocated memory is cleaned up in pmcstat_cleanup().
Memory calculation for default value of args.pa_kernel was one more than
necessary.

Test Plan

Run on x86 installs to ensure no runtime issues occur.

[jtl]

Generally looks fine. See in-line comments.

usr.sbin/pmcstat/pmcstat.c

203

It looks like this will result in ev being removed from the list twice (here and line 214).

807

If you're going to check this, you should probably also check the ev_spec strdup() call a few lines up.

[jhb]

usr.sbin/pmcstat/pmcstat.c

203

Yes, I would just leave this as a STAILQ_FOREACH_SAFE(). I think the real fix is moving the calls to free() and STAILQ_REMOVE() out of the PMC_ID_INVALID conditional.

204

FWIW, calling free on global vars during a clean shutdown isn't really that important. If you know you are going to exit() then it's going to get cleaned up anyway. I suppose this might appease valgrind, but it actually increases runtime and CPU cycles used.

[stevek]

usr.sbin/pmcstat/pmcstat.c

203

Yes, I'm guessing it was an accidental paste from vim when I had deleted the line after re-structuring the code.

204

True. The question is do we want to forego the cleanup of memory and just do the pmc_stop/pmc_release only, since all the cases that seem to call this function, we are getting out.

Or perhaps only do the cleanup if one is building for leak detection? (Is there a somewhat standard define for that? Like there is for lint?)

807

Good point.

[stevek]

Just remove freeing memory in pmcstat_cleanup, since we only clean up
when we are exiting anyway.

Add NULL checks for all malloc and strdup returns.

[gnn]

LGTM