Fix leaks and test for getpagesize() returning == -1
ClosedPublic
Actions

Authored by ngie on Apr 19 2016, 9:58 PM.

Details

Reviewers

cem

Summary

Fix leaks and test for getpagesize() returning == -1

close file descriptors after use.
Always munmap memory regions after mmap'ing them.
Make sure getpagesize() returns a value greater than 0 and use a cached value instead of always calling getpagesize(3).

MFC after: 2 weeks
Reported by: Coverity
CID: 1331374-1331377, 1331653-1331662
Sponsored by: EMC / Isilon Storage Division

Test Plan

sudo kyua test -k /usr/tests/sys/vm/Kyuafile

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 3331
Build 3366: arc lint + arc unit

Event Timeline

ngie updated this revision to Diff 15350.Apr 19 2016, 9:58 PM

ngie retitled this revision from to Fix leaks and test for getpagesize() returning == -1.

ngie updated this object.

ngie edited the test plan for this revision. (Show Details)

ngie added subscribers: cem, jhb, markj.

Herald added a subscriber: imp. · View Herald TranscriptApr 19 2016, 9:58 PM

ngie added a subscriber: tests.Apr 19 2016, 9:59 PM

ngie updated this object.Apr 19 2016, 10:06 PM

cem accepted this revision.Apr 19 2016, 10:13 PM

cem added a reviewer: cem.

cem added inline comments.

tests/sys/vm/mmap_test.c
257–258	What about `munmap`ing p1, p2, p3?

This revision is now accepted and ready to land.Apr 19 2016, 10:13 PM

ngie marked an inline comment as done.Apr 19 2016, 10:23 PM

ngie added inline comments.

tests/sys/vm/mmap_test.c
257–258	Good catch -- fixed :).

ngie marked 2 inline comments as done.Apr 19 2016, 10:24 PM

Ok I guess?

I've yet to see the ATF stuff ever run more than one test in a process. It seems to always want to start a new process to run each test, so doing the cleanups here is just slowing things down (which seems somewhat ironic given that you are micro-optimizing getpagesize() which will never fail).

In D6011#128048, @jhb wrote:

Ok I guess?

I've yet to see the ATF stuff ever run more than one test in a process. It seems to always want to start a new process to run each test, so doing the cleanups here is just slowing things down (which seems somewhat ironic given that you are micro-optimizing getpagesize() which will never fail).

True, there is a separate process for each ATF test. But I think the cleanup is worth it just to make Coverity shut up. It helps to find the real Coverity bugs, like the buffer overflow that cem fixed yesterday.

ngie closed this revision.Apr 30 2016, 9:38 AM