git: d85ea7e79b30 - stable/14 - lorder: Clean up and improve robustness.

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Mon, 11 Mar 2024 12:35:07 UTC
The branch stable/14 has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=d85ea7e79b30dd51281f2fb8a457e5eccc3fc47e

commit d85ea7e79b30dd51281f2fb8a457e5eccc3fc47e
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-02-28 15:37:36 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-03-11 12:19:06 +0000

    lorder: Clean up and improve robustness.
    
    * Properly parse (no) command-line options.
    
    * Ensure that errors go to stderr and result in a non-zero exit.
    
    * Drop the special-case code for a single argument, as it will produce
      the wrong outcome if the file does not exist or is corrupted.
    
    * Don't print anything until after we've collected all the data.
    
    * Always create all temporary files before setting the trap.  This
      ensures that the trap can safely fire at any moment, regardless of any
      previous definition of `T`.
    
    * Use a temporary file rather than a pipe between `nm` and `sed` to
      ensure proper termination if `nm` fails due to a missing or invalid
      input.
    
    * The check for self-referential entries was conditional on testing our
      argument list against a regex looking for archives.  This was a
      needless and unreliable optimization; make the check unconditional.
    
    * Document that lorder will not work properly if any of its inputs have
      spaces in their name.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    allanjude
    Differential Revision:  https://reviews.freebsd.org/D44133
    
    (cherry picked from commit 5c7b986c21ed47545203e8f630fe2281b83112d2)
    
    lorder: Add unit tests.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    allanjude
    Differential Revision:  https://reviews.freebsd.org/D44134
    
    (cherry picked from commit 96da41b6dbf383436018f11ad8a672faab2d3789)
    
    lorder: Undeprecate.
    
    While lorder is not required by our current toolchain (or any toolchain
    we've used in the past decade or two), it still occasionally shows up
    in build systems of third party software, including The Open Group's
    UNIX conformance test suite, and the maintenance cost is negligible.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    imp, allanjude, emaste
    Differential Revision:  https://reviews.freebsd.org/D44135
    
    (cherry picked from commit cf4d9bf8b38819da12c6d686d5cf6dbd6353cd61)
    
    lorder: Don't rely on legacy syntax.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    allanjude
    Differential Revision:  https://reviews.freebsd.org/D44155
    
    (cherry picked from commit aedb37dc49319a7cd1faf34f312a8a9fc01e611d)
    
    lorder: Tweak invalid file test.
    
    Different implementations of `nm` have different ways of telling you
    that your file is not a valid object or library, but they all seem to
    have “not recognized” as a common substring.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    allanjude
    Differential Revision:  https://reviews.freebsd.org/D44154
    
    (cherry picked from commit aa80cfadff0bb715ca090cbd1b3561a1619251d5)
---
 etc/mtree/BSD.tests.dist            |   2 +
 usr.bin/lorder/Makefile             |   4 ++
 usr.bin/lorder/lorder.1             |  37 ++++++------
 usr.bin/lorder/lorder.sh            |  94 ++++++++++++++++++------------
 usr.bin/lorder/tests/Makefile       |   4 ++
 usr.bin/lorder/tests/lorder_test.sh | 111 ++++++++++++++++++++++++++++++++++++
 6 files changed, 195 insertions(+), 57 deletions(-)

diff --git a/etc/mtree/BSD.tests.dist b/etc/mtree/BSD.tests.dist
index 8b985f3b4682..8a3b097c4162 100644
--- a/etc/mtree/BSD.tests.dist
+++ b/etc/mtree/BSD.tests.dist
@@ -1087,6 +1087,8 @@
         ..
         lockf
         ..
+        lorder
+        ..
         m4
         ..
         mkimg
diff --git a/usr.bin/lorder/Makefile b/usr.bin/lorder/Makefile
index a94860b51c6a..6b3fa2c829d6 100644
--- a/usr.bin/lorder/Makefile
+++ b/usr.bin/lorder/Makefile
@@ -1,6 +1,10 @@
 #	@(#)Makefile	8.1 (Berkeley) 6/6/93
+.include <src.opts.mk>
 
 SCRIPTS=lorder.sh
 MAN=	lorder.1
 
+HAS_TESTS=
+SUBDIR.${MK_TESTS}=	tests
+
 .include <bsd.prog.mk>
diff --git a/usr.bin/lorder/lorder.1 b/usr.bin/lorder/lorder.1
index e268f81d7254..478b7f493e03 100644
--- a/usr.bin/lorder/lorder.1
+++ b/usr.bin/lorder/lorder.1
@@ -27,17 +27,12 @@
 .\"
 .\"     @(#)lorder.1	8.2 (Berkeley) 4/28/95
 .\"
-.Dd March 21, 2023
+.Dd February 27, 2024
 .Dt LORDER 1
 .Os
 .Sh NAME
 .Nm lorder
 .Nd list dependencies for object files
-.Sh DEPRECATION NOTICE
-.Nm
-is obsolete and may not be present in
-.Fx 14
-and later.
 .Sh SYNOPSIS
 .Nm
 .Ar
@@ -46,13 +41,10 @@ The
 .Nm
 utility uses
 .Xr nm 1
-to determine interdependencies in the list of object files
-and library archives
-specified on the command line.
-The
-.Nm
-utility outputs a list of file names where the first file contains a symbol
-which is defined by the second file.
+to determine interdependencies between object files and library
+archives listed on its command line.
+It then outputs a list of pairs of file names such that the first file
+in each pair references at least one symbol defined by the second.
 .Pp
 The output is normally used with
 .Xr tsort 1
@@ -60,18 +52,16 @@ when a library is created to determine the optimum ordering of the
 object modules so that all references may be resolved in a single
 pass of the loader.
 .Pp
-When linking static binaries,
+Similarly, when linking static binaries,
 .Nm
 and
 .Xr tsort 1
-can be used to properly order library archives automatically.
+can be used to sort libraries in order of dependency.
 .Pp
-The use of
-.Nm
-is not required by contemporary linkers, and
-.Nm
-may be removed from a future version of
-.Fx .
+While contemporary linkers no longer require the use of
+.Nm ,
+it is provided for the benefit of legacy code bases and build
+systems which still insist on it.
 .Sh ENVIRONMENT
 .Bl -tag -width indent
 .It Ev NM
@@ -99,3 +89,8 @@ A
 .Nm
 utility appeared in
 .At v7 .
+.Sh CAVEATS
+The
+.Nm
+utility will not work properly if given file names with spaces or
+newlines in them.
diff --git a/usr.bin/lorder/lorder.sh b/usr.bin/lorder/lorder.sh
index 640e128b0cb9..c0a7dbbd43e6 100644
--- a/usr.bin/lorder/lorder.sh
+++ b/usr.bin/lorder/lorder.sh
@@ -33,35 +33,50 @@
 #
 #
 
-# only one argument is a special case, just output the name twice
-case $# in
-	0)
-		echo "usage: lorder file ...";
-		exit ;;
-	1)
-		echo $1 $1;
-		exit ;;
-esac
+export LC_CTYPE=C
+export LC_COLLATE=C
+set -e
 
-# temporary files
+usage() {
+	echo "usage: lorder file ..." >&2
+	exit 1
+}
+
+while getopts "" opt ; do
+	case $opt in
+	*)
+		usage
+		;;
+	esac
+done
+shift $(($OPTIND - 1))
+if [ $# -eq 0 ] ; then
+	usage
+fi
+
+#
+# Create temporary files.
+#
+N=$(mktemp -t _nm_)
 R=$(mktemp -t _reference_)
 S=$(mktemp -t _symbol_)
+T=$(mktemp -t _temp_)
 NM=${NM:-nm}
 
-# remove temporary files on HUP, INT, QUIT, PIPE, TERM
-trap "rm -f $R $S $T; exit 1" 1 2 3 13 15
-
-# make sure all the files get into the output
-for i in $*; do
-	echo $i $i
-done
+#
+# Remove temporary files on termination.
+#
+trap "rm -f $N $R $S $T" EXIT 1 2 3 13 15
 
-# if the line has " [RTDW] " it's a globally defined symbol, put it
-# into the symbol file.
 #
-# if the line has " U " it's a globally undefined symbol, put it into
-# the reference file.
-${NM} ${NMFLAGS} -go $* | sed "
+# A line matching " [RTDW] " indicates that the input defines a symbol
+# with external linkage; put it in the symbol file.
+#
+# A line matching " U " indicates that the input references an
+# undefined symbol; put it in the reference file.
+#
+${NM} ${NMFLAGS} -go "$@" >$N
+sed -e "
 	/ [RTDW] / {
 		s/:.* [RTDW] / /
 		w $S
@@ -72,21 +87,28 @@ ${NM} ${NMFLAGS} -go $* | sed "
 		w $R
 	}
 	d
-"
+" <$N
 
-export LC_ALL=C
-# eliminate references that can be resolved by the same library.
-if [ $(expr "$*" : '.*\.a[[:>:]]') -ne 0 ]; then
-	sort -u -o $S $S
-	sort -u -o $R $R
-	T=$(mktemp -t _temp_)
-	comm -23 $R $S >$T
-	mv $T $R
-fi
+#
+# Elide entries representing a reference to a symbol from within the
+# library that defines it.
+#
+sort -u -o $S $S
+sort -u -o $R $R
+comm -23 $R $S >$T
+mv $T $R
+
+#
+# Make sure that all inputs get into the output.
+#
+for i ; do
+	echo "$i" "$i"
+done
 
-# sort references and symbols on the second field (the symbol),
-# join on that field, and print out the file names.
+#
+# Sort references and symbols on the second field (the symbol), join
+# on that field, and print out the file names.
+#
 sort -k 2 -o $R $R
 sort -k 2 -o $S $S
-join -j 2 -o 1.1 2.1 $R $S
-rm -f $R $S
+join -j 2 -o 1.1 -o 2.1 $R $S
diff --git a/usr.bin/lorder/tests/Makefile b/usr.bin/lorder/tests/Makefile
new file mode 100644
index 000000000000..21207f413a8d
--- /dev/null
+++ b/usr.bin/lorder/tests/Makefile
@@ -0,0 +1,4 @@
+PACKAGE=	tests
+ATF_TESTS_SH=	lorder_test
+
+.include <bsd.test.mk>
diff --git a/usr.bin/lorder/tests/lorder_test.sh b/usr.bin/lorder/tests/lorder_test.sh
new file mode 100644
index 000000000000..a4276b2dcfe6
--- /dev/null
+++ b/usr.bin/lorder/tests/lorder_test.sh
@@ -0,0 +1,111 @@
+#
+# Copyright (c) 2024 Klara, Inc.
+#
+# SPDX-License-Identifier: BSD-2-Clause
+#
+
+atf_test_case noargs
+noargs_head() {
+	atf_set descr "No arguments"
+}
+noargs_body() {
+	atf_check -s exit:1 -e match:"^usage:" \
+		  lorder
+}
+
+atf_test_case onearg
+onearg_head() {
+	atf_set descr "One argument"
+}
+onearg_body() {
+	echo "void a(void) { }" >a.c
+	cc -o a.o -c a.c
+	echo "a.o a.o" >output
+	atf_check -o file:output \
+		  lorder *.o
+}
+
+atf_test_case dashdash
+dashdash_head() {
+	atf_set descr "One argument"
+}
+dashdash_body() {
+	echo "void a(void) { }" >a.c
+	cc -o a.o -c a.c
+	echo "a.o a.o" >output
+	atf_check -o file:output \
+		  lorder -- *.o
+}
+
+atf_test_case nonexistent
+nonexistent_head() {
+	atf_set descr "Nonexistent file"
+}
+nonexistent_body() {
+	atf_check -s not-exit:0 -e match:"No such file" -o empty \
+		  lorder nonexistent.o
+}
+
+atf_test_case invalid
+invalid_head() {
+	atf_set descr "Invalid file"
+}
+invalid_body() {
+	echo "not an object file" >invalid.o
+	atf_check -s not-exit:0 -e match:"not recognized" -o empty \
+		  lorder invalid.o
+}
+
+atf_test_case objects
+objects_head() {
+	atf_set descr "Order objects"
+}
+objects_body() {
+	echo "void a(void) { }" >a.c
+	echo "void a(void); void b(void) { a(); }" >b.c
+	echo "void b(void); void c(void) { b(); }" >c.c
+	for n in a b c ; do
+		cc -o $n.o -c $n.c
+		echo "$n.o $n.o"
+	done >output
+	echo "b.o a.o" >>output
+	echo "c.o b.o" >>output
+	atf_check -o file:output \
+		  lorder *.o
+}
+
+atf_test_case archives
+archives_head() {
+	atf_set descr "Order archives"
+}
+archives_body() {
+	echo "void a(void) { }" >a.c
+	echo "void a(void); void b(void) { a(); }" >b.c
+	echo "void b(void); void c(void) { b(); }" >c.c
+	echo "void e(void); void d(void) { e(); }" >d.c
+	echo "void d(void); void e(void) { d(); }" >e.c
+	for n in a b c d e ; do
+		cc -o $n.o -c $n.c
+	done
+	for n in a b c ; do
+		ar -crs $n.a $n.o
+		echo "$n.a $n.a"
+	done >output
+	ar -crs z.a d.o e.o
+	echo "z.a z.a" >>output
+	echo "b.a a.a" >>output
+	echo "c.a b.a" >>output
+	atf_check -o file:output \
+		  lorder *.a
+}
+
+atf_init_test_cases()
+{
+	atf_add_test_case noargs
+	atf_add_test_case onearg
+	atf_add_test_case dashdash
+	atf_add_test_case nonexistent
+	atf_add_test_case invalid
+	atf_add_test_case objects
+	atf_add_test_case archives
+}