Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Improve install -s
To:
tech@openbsd.org
Date:
Fri, 21 Aug 2026 20:16:07 +0200

Download raw body.

Thread
  • Mark Kettenis:

    Improve install -s

The -s option runs strip(1) on the installed binary.  This means it
first copies the unstripped binary to a temporary file, strips that
temporary file and then moves the stripped file into place.

This is a bit stupid as strip(1) effectively does another copy.  The
diff below makes use of the "-o" option to avoid that step. This
avoids the extra copy.  Which also means you need less free space on
the destination filesystem.

Also check the exit status of the strip command and exit if it isn't
zero.

This might break the case where somebody sets STRIP to run a command
that doesn't implement the "-o" option in a compatible manner.  I'd
say you get to keep bboth pieces in that case.

Thoughts?


Index: usr.bin/xinstall/xinstall.c
===================================================================
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
diff -u -p -r1.78 xinstall.c
--- usr.bin/xinstall/xinstall.c	17 Oct 2024 15:38:38 -0000	1.78
+++ usr.bin/xinstall/xinstall.c	21 Aug 2026 18:09:13 -0000
@@ -72,7 +72,7 @@ void	copy(int, char *, int, char *, off_
 int	compare(int, const char *, off_t, int, const char *, off_t);
 void	install(char *, char *, u_long, u_int);
 void	install_dir(char *, int);
-void	strip(char *);
+void	strip(char *, char *);
 void	usage(void);
 int	create_tempfile(char *, char *, size_t);
 int	file_write(int, char *, size_t, int *, int *, int);
@@ -256,7 +256,7 @@ install(char *from_name, char *to_name, 
 		docompare = 0;
 	}
 
-	if (!devnull) {
+	if (!devnull && !dostrip) {
 		if ((from_fd = open(from_name, O_RDONLY)) == -1)
 			err(1, "%s", from_name);
 	}
@@ -265,12 +265,12 @@ install(char *from_name, char *to_name, 
 	if (to_fd < 0)
 		err(1, "%s", tempfile);
 
-	if (!devnull)
+	if (!devnull && !dostrip)
 		copy(from_fd, from_name, to_fd, tempfile, from_sb.st_size,
 		    ((off_t)from_sb.st_blocks * S_BLKSIZE < from_sb.st_size));
 
 	if (dostrip) {
-		strip(tempfile);
+		strip(from_name, tempfile);
 
 		/*
 		 * Re-open our fd on the target, in case we used a strip
@@ -532,7 +532,7 @@ compare(int from_fd, const char *from_na
  *	use strip(1) to strip the target file
  */
 void
-strip(char *to_name)
+strip(char *from_name, char *to_name)
 {
 	int serrno, status;
 	char * volatile path_strip;
@@ -547,7 +547,8 @@ strip(char *to_name)
 		(void)unlink(to_name);
 		errc(1, serrno, "forks");
 	case 0:
-		execl(path_strip, "strip", "--", to_name, (char *)NULL);
+		execl(path_strip, "strip", "-o", to_name, "--", from_name,
+		    (char *)NULL);
 		warn("%s", path_strip);
 		_exit(1);
 	default:
@@ -555,8 +556,10 @@ strip(char *to_name)
 			if (errno != EINTR)
 				break;
 		}
-		if (!WIFEXITED(status))
+		if (!WIFEXITED(status) || WEXITSTATUS(status)) {
 			(void)unlink(to_name);
+			err(1, "strip");
+		}
 	}
 }