Paths

Table of Contentst

Differential D31402

ar: add error handling
ClosedPublic
Actions

Authored by emaste on Aug 3 2021, 6:39 PM.

Tags

None

Referenced Files

	F107385233: D31402.id93190.diff
	Mon, Jan 13, 9:46 AM

	F107384962: D31402.id93504.diff
	Mon, Jan 13, 9:40 AM

	F107384349: D31402.id93210.diff
	Mon, Jan 13, 9:26 AM

	F107379171: D31402.diff
	Mon, Jan 13, 7:41 AM

	Unknown Object (File)
	Mon, Jan 6, 5:14 AM

	Unknown Object (File)
	Mon, Jan 6, 12:22 AM

	Unknown Object (File)
	Dec 5 2024, 9:26 AM

	Unknown Object (File)
	Nov 13 2024, 8:06 PM

Subscribers

Details

Reviewers

Commits

rGfb15a9f90b29: ar: provide error exit status upon failure
rGd71103026de3: ar: provide error exit status upon failure
rG38911b3c2c7d: ar: provide error exit status upon failure

Summary

Previously ar and ranlib did not produce an error exit status in the case of a misisng file etc.

Bring over the error handling added by ELF Tool Chain after forking FreeBSD's ar.

Reported by: HardenedBSD IRC
Obtained from: elftoolchain ar
Sponsored by: The FreeBSD Foundation

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

emaste requested review of this revision.Aug 3 2021, 6:39 PM

emaste created this revision.

Reported by: gehmehgeh on HardenedBSD IRC

Exp-run in PR257599

Oh, actually:
Reported by: Shawn Webb

(https://lists.freebsd.org/archives/freebsd-toolchain/2021-August/000121.html)
I missed the mailing list post originally

markj added a subscriber: markj.Aug 4 2021, 3:08 AM

markj added inline comments.

usr.bin/ar/acpyacc.y
194	Looks like this is just a related cleanup? There is a similar change in acplex.l that is not included here, EX_SOFTWARE -> EXIT_FAILURE.
662	There are quite a few more sysexits.h removals in upstream ar, not sure if the intent was to port them all?
usr.bin/ar/ar.c
283	EXIT_FAILURE here too? Or should we go further and eliminate the exit status parameter from bsdar_errc() and hard-code EXIT_FAILURE, like elftoolchain did?
351	None of these calls are checked?

emaste added inline comments.Aug 4 2021, 3:34 AM

usr.bin/ar/acpyacc.y
194	Right, I just compared usr.bin/ar with elftc files and updated exit status related parts (that I found) to match elftc. acpyacc.y could indeed be left unchanged, but I think converging on elftc is preferable.
662	Not a bad idea, perhaps a second pass.

Improvements from @markj

LGTM aside from the one comment.

usr.bin/ar/read.c
163	Upstream ar returns EXIT_FAILURE in this case (`r == ARCHIVE_FATAL` check at the end of ar_read_archive()).

This revision is now accepted and ready to land.Aug 4 2021, 12:28 PM

One more fix from markj
Commit pending exp-run results

This revision now requires review to proceed.Aug 4 2021, 12:53 PM

markj accepted this revision.Aug 4 2021, 12:54 PM

This revision is now accepted and ready to land.Aug 4 2021, 12:54 PM

exp-run turned up one failing port, math/lapacke; I have a fix in D31492

emaste mentioned this in R11:6fd5f57d0613: math/lapack: remove superfluous and broken ranlib invocation.Aug 10 2021, 9:07 PM

Closed by commit rG38911b3c2c7d: ar: provide error exit status upon failure (authored by emaste). · Explain WhyAug 10 2021, 9:12 PM

This revision was automatically updated to reflect the committed changes.

emaste added a commit: rG38911b3c2c7d: ar: provide error exit status upon failure.

emaste added a commit: rGd71103026de3: ar: provide error exit status upon failure.Oct 9 2021, 12:29 AM

Herald added a subscriber: imp. · View Herald TranscriptOct 9 2021, 12:29 AM

emaste added a commit: rGfb15a9f90b29: ar: provide error exit status upon failure.Oct 9 2021, 12:30 AM

Revision Contents
Changeset List

Path

Size

usr.bin/

ar/

4 lines

6 lines

20 lines

61 lines

36 lines

49 lines

Diff 93223

usr.bin/ar/acplex.l

Loading...

usr.bin/ar/acpyacc.y

Loading...

usr.bin/ar/ar.h

Loading...

usr.bin/ar/ar.c

Loading...

usr.bin/ar/read.c

Loading...

usr.bin/ar/write.c

Loading...