Page MenuHomeFreeBSD
Differential D38280

cmp: Print a summary on SIGINFO.
ClosedPublic
Actions

Authored by des on Jan 30 2023, 5:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 13, 6:34 PM
Unknown Object (File)
Mon, Jan 13, 3:14 PM
Unknown Object (File)
Dec 11 2024, 2:35 AM
Unknown Object (File)
Nov 26 2024, 9:32 PM
Unknown Object (File)
Nov 26 2024, 9:31 PM
Unknown Object (File)
Nov 26 2024, 9:30 PM
Unknown Object (File)
Nov 26 2024, 9:30 PM
Unknown Object (File)
Nov 26 2024, 8:42 PM
View All 70 Files
Subscribers
imp
markj

Details

Reviewers
markj
Group Reviewers
Klara
Commits
rG54117b169fba: cmp: Print a summary on SIGINFO.
rG6673a5476d02: cmp: Print a summary on SIGINFO.
Summary

MFC after: 1 week
Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 49333
Build 46223: arc lint + arc unit

Event Timeline

des created this revision.Jan 30 2023, 5:26 PM
Herald added a subscriber: imp. · View Herald TranscriptJan 30 2023, 5:26 PM
des requested review of this revision.Jan 30 2023, 5:26 PM
Harbormaster completed remote builds in B49328: Diff 116053.Jan 30 2023, 5:26 PM
des added a child revision: D38281: cmp: Increase buffer size for non-mmap case..Jan 30 2023, 5:26 PM
des added a reviewer: Klara.Jan 30 2023, 5:29 PM
markj added a subscriber: markj.Jan 30 2023, 5:39 PM
markj added inline comments.
usr.bin/cmp/cmp.c
255

If you don't intend to check for errors. It's good for silencing static analyzers if nothing else.

271

Why bother unsetting it?

usr.bin/cmp/regular.c
126

I think it's usually preferable to print these to stderr, lest one inject something unexpected into a pipe.

des updated this revision to Diff 116060.Jan 30 2023, 6:22 PM
des marked 3 inline comments as done.

Incorporate reviewer feedback.

usr.bin/cmp/cmp.c
271

Good point, it's harmless.

usr.bin/cmp/regular.c
126

You're right, I misremembered.

Harbormaster completed remote builds in B49333: Diff 116060.Jan 30 2023, 6:22 PM
markj accepted this revision as: markj.Jan 30 2023, 6:58 PM

It's not obvious to me why we bother with the ifdefs. Most other utilities in the base system don't conditionalize SIGINFO support, but I guess it doesn't hurt.

This revision is now accepted and ready to land.Jan 30 2023, 6:58 PM
des added a comment.EditedJan 30 2023, 11:44 PM
In D38280#869740, @markj wrote:

It's not obvious to me why we bother with the ifdefs. Most other utilities in the base system don't conditionalize SIGINFO support, but I guess it doesn't hurt.

Well, extern.h needs the #ifdef unless we want to #include <signal.h> in link.c and misc.c, which don't need it, and the remaining #ifdefs a) don't hurt and b) make the code easier for others to pull if they want to. I initially also put in a bit that defined SIGINFO to SIGVTALRM if the latter is defined but the former is not but took it out.

Closed by commit rG6673a5476d02: cmp: Print a summary on SIGINFO. (authored by des). · Explain WhyJan 31 2023, 12:29 AM
This revision was automatically updated to reflect the committed changes.
des added a commit: rG6673a5476d02: cmp: Print a summary on SIGINFO..
des added a commit: rG54117b169fba: cmp: Print a summary on SIGINFO..Feb 9 2023, 8:41 PM

Revision Contents
Changeset List

PathSize
usr.bin/
cmp/
14 lines
4 lines
7 lines
7 lines

Diff 116060

View Options

usr.bin/cmp/cmp.c

Loading...
View Options

usr.bin/cmp/extern.h

Loading...
View Options

usr.bin/cmp/regular.c

Loading...
View Options

usr.bin/cmp/special.c

Loading...