Page MenuHomeFreeBSD

Implement simple record boundary tracking in sbuf(9)
ClosedPublic

Authored by lstewart on Nov 16 2016, 8:59 AM.

Details

Summary

Implement simple record boundary tracking in sbuf(9) for use with drain operations to avoid record splitting.

Sponsored by: Netflix, Inc.

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

lstewart updated this revision to Diff 22248.Nov 16 2016, 8:59 AM
lstewart retitled this revision from to Implement simple record boundary tracking in sbuf(9).
lstewart updated this object.
lstewart edited the test plan for this revision. (Show Details)
lstewart added a reviewer: ian.
lstewart set the repository for this revision to rS FreeBSD src repository.
lstewart updated this object.Nov 16 2016, 9:25 AM
lstewart edited edge metadata.
imp accepted this revision.Nov 16 2016, 3:16 PM
imp added a reviewer: imp.

This looks good to me, but I'd get another set of eyes to approve as well before the commit.

This revision is now accepted and ready to land.Nov 16 2016, 3:16 PM
cem added a subscriber: cem.Nov 16 2016, 6:56 PM

Looks fine, but I'd like to see a diff with context first. Can you upload a -U9999 diff?

wblock added a subscriber: wblock.Nov 18 2016, 5:12 PM
wblock added inline comments.
share/man/man9/sbuf.9
189 ↗(On Diff #22248)

Passive->active: s/will be set/is set/

lstewart updated this revision to Diff 31737.Aug 8 2017, 12:59 AM
lstewart edited edge metadata.
lstewart removed a reviewer: ian.

Rebase patch against current head and address review feedback.

This revision now requires review to proceed.Aug 8 2017, 12:59 AM
lstewart marked an inline comment as done.Aug 8 2017, 1:00 AM
cem accepted this revision.Aug 16 2017, 9:38 PM

Looks fine. I think it would be good to note in the commit message that this is only for top-level sections and does not nest. It's obvious when you think about it, but, I like clarity.

This revision is now accepted and ready to land.Aug 16 2017, 9:38 PM
In D8536#249937, @cem wrote:

Looks fine. I think it would be good to note in the commit message that this is only for top-level sections and does not nest. It's obvious when you think about it, but, I like clarity.

Thanks very much for the feedback and review Conrad (and apologies for the long hiatus in circling back to wrap this up). Will include details in commit log as you suggest. To clarify, is the man page text sufficiently clear?

cem added a comment.Aug 17 2017, 4:13 AM

Yes, I think the man page additions are great.

This revision was automatically updated to reflect the committed changes.