Page MenuHomeFreeBSD

Fix memory leaks during "tail -r" of a non-regular file
ClosedPublic

Authored by asomers on Jan 6 2017, 6:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 7, 3:50 PM
Unknown Object (File)
Nov 12 2024, 3:36 AM
Unknown Object (File)
Nov 12 2024, 1:29 AM
Unknown Object (File)
Oct 15 2024, 5:35 AM
Unknown Object (File)
Oct 5 2024, 1:38 AM
Unknown Object (File)
Sep 30 2024, 12:03 AM
Unknown Object (File)
Sep 30 2024, 12:01 AM
Unknown Object (File)
Sep 30 2024, 12:01 AM
Subscribers

Details

Summary

Fix memory leaks during "tail -r" of a non-regular file

  • Rewrite r_buf to use standard tail queues instead of a hand-rolled circular linked list. Free dynamic allocations when done.
  • Remove an optimization for the case where the file is a multiple of 128KB in size and there is a scarcity of memory.
  • Add ATF tests for "tail -r" and its variants.
Test Plan

Added ATF tests

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

asomers retitled this revision from to Fix memory leaks during "tail -r" of a non-regular file.
asomers updated this object.
asomers edited the test plan for this revision. (Show Details)
asomers added a reviewer: ngie.
usr.bin/tail/reverse.c
193 ↗(On Diff #23660)

Might be a good idea to initialize this to NULL to ensure someone doesn't add a goto that will free a bogus memory allocation.

193 ↗(On Diff #23660)

this -> first.

201 ↗(On Diff #23660)

Whitespace is off here.

207–213 ↗(On Diff #23660)

This whitespace looks off.

219 ↗(On Diff #23660)

Whitespace looks off here.

246 ↗(On Diff #23660)

while (tl != NULL) {

248 ↗(On Diff #23660)

Should this be readded (well.. an adapted version), just in case?

260 ↗(On Diff #23660)

if (tr != NULL) {?

usr.bin/tail/tests/tail_test.sh
118 ↗(On Diff #23660)

tail -r < infile > outpipe instead?

134 ↗(On Diff #23660)

Should check the result of this command to make sure this succeeds.

143–144 ↗(On Diff #23660)

Probably should check diskspace beforehand to make sure there's enough, right?

163 ↗(On Diff #23660)

tail -r < infile > outpipe instead?

179 ↗(On Diff #23660)

Same here with using cat...

195 ↗(On Diff #23660)

Why /bin/echo ?

usr.bin/tail/reverse.c
248 ↗(On Diff #23660)

I'm not sure how. fread can't return EOF. You must use feof(3) instead, which is what I do.

usr.bin/tail/tests/tail_test.sh
134 ↗(On Diff #23660)

good catch

195 ↗(On Diff #23660)

Reflexes. Earlier versions of bash didn't implement "-n" in the echo builtin. But it looks like our sh has no such problem.

Incorporate ngie's feedback

ngie edited edge metadata.
This revision is now accepted and ready to land.Jan 7 2017, 8:14 AM
This revision was automatically updated to reflect the committed changes.