Page MenuHomeFreeBSD
Differential D36948

sort: replace home made line reader by getline(3)
ClosedPublic
Actions

Authored by bapt on Oct 12 2022, 1:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Nov 12 2024, 9:33 PM
Unknown Object (File)
Nov 7 2024, 11:16 PM
Unknown Object (File)
Oct 6 2024, 6:36 PM
Unknown Object (File)
Sep 16 2024, 4:22 PM
Unknown Object (File)
Sep 12 2024, 10:40 PM
Unknown Object (File)
Sep 8 2024, 2:41 PM
Unknown Object (File)
Sep 8 2024, 10:05 AM
Unknown Object (File)
Sep 5 2024, 6:28 AM
View All 56 Files
Subscribers
imp

Details

Reviewers
kevans
markj
Commits
rG489c9df19851: sort: replace home made line reader by getdelim(3)
rGb58094c0d98e: sort: replace home made line reader by getdelim(3)
Summary

The previous code had bug when reading lines with an unexpected
encoding, returning without the full line being captured.
This result in sort complaining with "sort: Illegal byte sequence"

Using getline(3) instead of the home made code, fixes the situation.

PR: 241679

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 47788
Build 44675: arc lint + arc unit

Event Timeline

bapt created this revision.Oct 12 2022, 1:36 PM
Herald added a subscriber: imp. · View Herald TranscriptOct 12 2022, 1:36 PM
bapt requested review of this revision.Oct 12 2022, 1:36 PM
Harbormaster completed remote builds in B47786: Diff 111689.Oct 12 2022, 1:36 PM
bapt added a reviewer: markj.Oct 12 2022, 1:49 PM
imp added inline comments.Oct 12 2022, 2:49 PM
usr.bin/sort/file.c
790

how is sort_opts_vals.zflag == 1 handled?

bapt updated this revision to Diff 111694.Oct 12 2022, 3:04 PM

use getdelim(3) to be able to respect the -z flag

Harbormaster completed remote builds in B47788: Diff 111694.Oct 12 2022, 3:04 PM
bapt marked an inline comment as done.Oct 12 2022, 3:05 PM
markj added a comment.Oct 12 2022, 3:05 PM

Seems ok, I wrote a regression test case based on the PR. I'll commit it after this lands.

usr.bin/sort/file.c
78

Indentation is off.

788–797

I thought this would break -z, but I wrote a test case and it seems to work with this patch. I don't understand why.

bapt updated this revision to Diff 111695.Oct 12 2022, 3:07 PM

Fix indentation

Harbormaster completed remote builds in B47789: Diff 111695.Oct 12 2022, 3:07 PM
bapt marked 2 inline comments as done.Oct 12 2022, 3:08 PM
imp added inline comments.Oct 12 2022, 3:10 PM
usr.bin/sort/file.c
795

Do you need to do this when using the -z flag?

bapt updated this revision to Diff 111696.Oct 12 2022, 3:18 PM

address @imp last comment

bapt marked an inline comment as done.Oct 12 2022, 3:19 PM
Harbormaster completed remote builds in B47790: Diff 111696.Oct 12 2022, 3:19 PM
markj accepted this revision.Oct 12 2022, 3:28 PM
markj added inline comments.
usr.bin/sort/file.c
788–797
This revision is now accepted and ready to land.Oct 12 2022, 3:28 PM
Closed by commit rGb58094c0d98e: sort: replace home made line reader by getdelim(3) (authored by bapt). · Explain WhyOct 12 2022, 3:38 PM
This revision was automatically updated to reflect the committed changes.
bapt added a commit: rGb58094c0d98e: sort: replace home made line reader by getdelim(3).
bapt added a commit: rG489c9df19851: sort: replace home made line reader by getdelim(3).Oct 19 2022, 8:02 AM

Revision Contents
Changeset List

PathSize
usr.bin/
sort/
109 lines
18 lines

Diff 111694

View Options

usr.bin/sort/bwstring.c

Loading...
View Options

usr.bin/sort/file.c

Loading...