Page MenuHomeFreeBSD

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

Authored by kevans on Aug 24 2022, 2:20 AM.

Details

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
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

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

pauamma added a subscriber: pauamma.
pauamma added inline comments.
usr.bin/split/split.1
215–219

Make it more obvious that the restriction was removed.

This revision now requires changes to proceed.Aug 25 2022, 11:09 PM
kevans marked an inline comment as done.

Address review commente

usr.bin/split/split.1
219

Did you mean to add something after "and"?

usr.bin/split/split.1
219

whoops: "the -l flag's line counting"

usr.bin/split/split.c
280–281

why not use getline(3) here?

pauamma 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
kevans marked an inline comment as done.

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

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

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