Details

Reviewers

bapt
emaste
pauamma_gundo.com

Group Reviewers

Klara
manpages

Commits

rG5c053aa3c5e9: split: switch to getline() for line/pattern matching

Summary

split: switch to getline() for line/pattern matching

Get rid of split's home-grown logic for growing the buffer; arbitrarily
breaking at LONG_MAX bytes instead of 65536 bytes gives us much more
wiggle room.  Additionally, we'll actually fail out entirely if we can't
fit a line, which makes noticing this class of problem much easier.

Sponsored by:   Klara, Inc.
Differential Revision:  https://reviews.freebsd.org/D36323

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 47098
Build 43985: arc lint + arc unit

Event Timeline

kevans created this revision.Aug 24 2022, 2:20 AM

Herald added a reviewer: manpages. · View Herald TranscriptAug 24 2022, 2:20 AM

Herald added a subscriber: imp. · View Herald Transcript

kevans requested review of this revision.Aug 24 2022, 2:20 AM

kevans added a child revision: D36324: split: add some tests.

Harbormaster completed remote builds in B47098: Diff 109751.Aug 24 2022, 2:21 AM

Thanks for working on this, LGTM for the man page part.

pauamma_gundo.com requested changes to this revision.Aug 25 2022, 11:09 PM

pauamma_gundo.com added a subscriber: pauamma_gundo.com.

pauamma_gundo.com added inline comments.

usr.bin/split/split.1
215	Make it more obvious that the restriction was removed.

This revision now requires changes to proceed.Aug 25 2022, 11:09 PM

Address review commente

Harbormaster completed remote builds in B47214: Diff 110035.Sep 1 2022, 4:47 AM

pauamma_gundo.com added inline comments.Sep 6 2022, 9:10 PM

usr.bin/split/split.1
218	Did you mean to add something after "and"?

kevans added inline comments.Sep 6 2022, 11:15 PM

usr.bin/split/split.1
218	whoops: "the -l flag's line counting"

bapt added inline comments.Oct 11 2022, 11:48 AM

usr.bin/split/split.c
324–332	why not use getline(3) here?

pauamma_gundo.com accepted this revision.Oct 11 2022, 9:39 PM

pauamma_gundo.com added inline comments.

usr.bin/split/split.1
31	Bump on commit.

This revision is now accepted and ready to land.Oct 11 2022, 9:39 PM

Switch to getline() for split2's buffer management, move the global buf/bufsize
into split1() and keep that at MAXBSIZE.

This revision now requires review to proceed.Oct 25 2022, 3:35 AM

Harbormaster completed remote builds in B47984: Diff 112174.Oct 25 2022, 3:36 AM

Clean up the diff a bit more; don't rename bfr -> buf for split1(), but maintain
the MAXBSIZE -> sizeof(bfr) change.

Harbormaster completed remote builds in B47985: Diff 112175.Oct 25 2022, 3:38 AM

kevans retitled this revision from split: use a dynamic buffer for -l/-p matching to split: switch to getline() for line/pattern matching.Oct 25 2022, 3:39 AM

kevans edited the summary of this revision. (Show Details)

Manual page change looks good and consistent with the change summary.

This revision is now accepted and ready to land.Oct 25 2022, 10:21 AM

bapt accepted this revision.Oct 25 2022, 12:42 PM

emaste accepted this revision.Oct 25 2022, 1:18 PM

Closed by commit rG5c053aa3c5e9: split: switch to getline() for line/pattern matching (authored by kevans). · Explain WhyOct 25 2022, 3:06 PM

This revision was automatically updated to reflect the committed changes.

kevans added a commit: rG5c053aa3c5e9: split: switch to getline() for line/pattern matching.