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.
Details
Details
Run on x86 installs to ensure no runtime issues occur.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Generally looks fine. See in-line comments.
| usr.sbin/pmcstat/pmcstat.c | ||
|---|---|---|
| 201 ↗ | (On Diff #20604) | It looks like this will result in ev being removed from the list twice (here and line 214). |
| 816 ↗ | (On Diff #20604) | If you're going to check this, you should probably also check the ev_spec strdup() call a few lines up. |
| usr.sbin/pmcstat/pmcstat.c | ||
|---|---|---|
| 201 ↗ | (On Diff #20604) | 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. |
| 218 ↗ | (On Diff #20604) | 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. |
| usr.sbin/pmcstat/pmcstat.c | ||
|---|---|---|
| 201 ↗ | (On Diff #20604) | Yes, I'm guessing it was an accidental paste from vim when I had deleted the line after re-structuring the code. |
| 218 ↗ | (On Diff #20604) | 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?) |
| 816 ↗ | (On Diff #20604) | Good point. |
Comment Actions
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.