Paths

Table of Contentst

Differential D38496

wc: Clean up and modernize.
ClosedPublic
Actions

Authored by des on Feb 10 2023, 5:49 PM.

Tags

None

Referenced Files

	F168158131: D38496.id116961.diff
	Wed, Aug 26, 3:44 PM

	Unknown Object (File)
	Tue, Aug 25, 3:52 PM

	Unknown Object (File)
	Tue, Aug 25, 10:03 AM

	Unknown Object (File)
	Tue, Aug 25, 9:07 AM

	Unknown Object (File)
	Tue, Aug 25, 9:06 AM

	Unknown Object (File)
	Tue, Aug 25, 4:21 AM

	Unknown Object (File)
	Tue, Aug 25, 2:39 AM

	Unknown Object (File)
	Tue, Aug 25, 1:32 AM

Subscribers

Details

Reviewers

ngie
kevans

Group Reviewers

Klara
tests

Commits

rGdad64f0e7cad: wc: Clean up and modernize.

Summary

Drop <err.h>, which is unnecessary since we use libxo.

As per POSIX, report an error if output fails.

Fix some type mismatches.

Use bool instead of int where appropriate.

Avoid repeatedly checking for a null filename.

Miscellaneous other tidying.

Add tests (partly derived from work performed by SHENG-YI HONG <i19780219111@kimo.com>).

Sponsored by: Klara, Inc.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 49676
Build 46566: arc lint + arc unit

Event Timeline

des created this revision.Feb 10 2023, 5:49 PM

Herald added a subscriber: imp. · View Herald TranscriptFeb 10 2023, 5:49 PM

des requested review of this revision.Feb 10 2023, 5:49 PM

Harbormaster completed remote builds in B49672: Diff 116953.Feb 10 2023, 5:49 PM

des mentioned this in D38489: wc: Drop <err.h> and report an error if xo_finish() fails..Feb 10 2023, 5:50 PM

Avoid repeatedly checking for a null filename.

Harbormaster completed remote builds in B49673: Diff 116954.Feb 10 2023, 6:12 PM

des edited the summary of this revision. (Show Details)Feb 10 2023, 6:12 PM

Switch mbrtowc() cases around to avoid awkward line wrapping.

Harbormaster completed remote builds in B49674: Diff 116955.Feb 10 2023, 6:14 PM

Avoid needless repetitions of a hardcoded buffer size.

Harbormaster completed remote builds in B49675: Diff 116956.Feb 10 2023, 6:16 PM

Add tests.

Harbormaster completed remote builds in B49676: Diff 116960.Feb 10 2023, 7:21 PM

Remove unintended test file.

Harbormaster completed remote builds in B49677: Diff 116961.Feb 10 2023, 7:22 PM

des edited the summary of this revision. (Show Details)Feb 10 2023, 7:22 PM

Fix multiline_repeated test.

Harbormaster completed remote builds in B49678: Diff 116962.Feb 10 2023, 7:27 PM

Fix exit codes.

Harbormaster completed remote builds in B49679: Diff 116964.Feb 10 2023, 7:49 PM

*sigh* I keep finding more stuff to fix

Harbormaster completed remote builds in B49680: Diff 116965.Feb 10 2023, 7:57 PM

I also have some WIP tests for wc(1) to be added in D35877

gbe added a subscriber: gbe.Feb 12 2023, 3:34 PM

Use printf(1) instead of echo.
Add a test case with only blank lines.
Add a test case with only whitespace.

Harbormaster completed remote builds in B49735: Diff 117096.Feb 13 2023, 3:45 PM

In D38496#876169, @lwhsu wrote:

I also have some WIP tests for wc(1) to be added in D35877

It's unfortunate that this wasn't approved back in the day. Does it cover anything that my tests don't?

des added reviewers: tests, ngie.Feb 13 2023, 3:51 PM

In D38496#877177, @des wrote:

In D38496#876169, @lwhsu wrote:

I also have some WIP tests for wc(1) to be added in D35877

It's unfortunate that this wasn't approved back in the day. Does it cover anything that my tests don't?

It's my fault that haven't completed it yet but I do plan to work on it with the submitter this week. Do you mind if I go ahead to remove the duplicated ones in D35877 and merge it first? Or you want to get this differential in first? I proposed to add tests first because it would be a good practice for code refactoring, but I can wait if you want.

Add descriptions to test cases.

Harbormaster completed remote builds in B49739: Diff 117104.Feb 13 2023, 5:13 PM

In D38496#877219, @lwhsu wrote:

In D38496#877177, @des wrote:

In D38496#876169, @lwhsu wrote:

I also have some WIP tests for wc(1) to be added in D35877

It's unfortunate that this wasn't approved back in the day. Does it cover anything that my tests don't?

It's my fault that haven't completed it yet but I do plan to work on it with the submitter this week. Do you mind if I go ahead to remove the duplicated ones in D35877 and merge it first? Or you want to get this differential in first? I proposed to add tests first because it would be a good practice for code refactoring, but I can wait if you want.

How about we add a comment to wc_test.sh crediting the student?

des edited the summary of this revision. (Show Details)Feb 13 2023, 8:03 PM

kevans accepted this revision as: Klara, kevans.Feb 13 2023, 8:50 PM

This revision is now accepted and ready to land.Feb 13 2023, 8:50 PM

Add a few more test and fix the whitespace test.

This revision now requires review to proceed.Feb 14 2023, 11:47 AM

Harbormaster completed remote builds in B49772: Diff 117194.Feb 14 2023, 11:47 AM

Nit: alphabetize envars.

Harbormaster completed remote builds in B49778: Diff 117202.Feb 14 2023, 2:39 PM

kevans accepted this revision as: Klara, kevans.Feb 14 2023, 7:18 PM

This revision is now accepted and ready to land.Feb 14 2023, 7:18 PM

Closed by commit rGdad64f0e7cad: wc: Clean up and modernize. (authored by des). · Explain WhyFeb 14 2023, 10:27 PM

This revision was automatically updated to reflect the committed changes.

des added a commit: rGdad64f0e7cad: wc: Clean up and modernize..

Revision Contents
Changeset List

Path

Size

etc/

mtree/

2 lines

usr.bin/

wc/

3 lines

tests/

8 lines

107 lines

110 lines

Diff 116960

etc/mtree/BSD.tests.dist

Loading...

usr.bin/wc/Makefile

Loading...

usr.bin/wc/foo

Loading...

usr.bin/wc/tests/Makefile

Loading...

usr.bin/wc/tests/wc_test.sh

Loading...

usr.bin/wc/wc.c

Loading...