Page MenuHomeFreeBSD

Fix a memory leak with add_mapping
ClosedPublic

Authored by trix_juniper.net on Feb 18 2017, 2:08 PM.

Details

Summary

The memory assigned to the local variable 'copy' is mostly freed.
The case where is can not be freed is noted with a comment.

Test Plan

Run clang's static analyzer scan-build before to find the problem. Run scan-build after to show the problem is reduced to the use of 'copy' in the badmopt: location.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

trix_juniper.net retitled this revision from to Fix a memory leak with add_mapping.
trix_juniper.net updated this object.
trix_juniper.net edited the test plan for this revision. (Show Details)
trix_juniper.net added reviewers: stevek, ed.
trix_juniper.net set the repository for this revision to rS FreeBSD src repository.
ed added inline comments.Feb 18 2017, 11:18 PM
usr.bin/tset/map.c
85 ↗(On Diff #25350)

I don't understand this change. What's the advantage of calling free() if you're already going to call errx()?

ed added inline comments.Feb 19 2017, 4:05 PM
usr.bin/tset/map.c
178 ↗(On Diff #25378)

Could you please move the call to free() closer to to the site where the variable is last used? Thanks!

moved free(copy) to last use of copy and poisoned copy after the free

ed accepted this revision.Feb 20 2017, 3:27 PM
ed edited edge metadata.
ed added inline comments.
usr.bin/tset/map.c
161 ↗(On Diff #25422)

I think assignments like these are typically not done in our codebase, especially not for local variables. Statements like these tend to have no effect, as the compiler will optimize it out anyway.

Apart from that, it looks good!

This revision is now accepted and ready to land.Feb 20 2017, 3:27 PM
This revision was automatically updated to reflect the committed changes.