Currently, install(1) deletes files when you are installing them to the
same directory, if you only specify the directory name and not the full
path:

  $ echo foo > foo; install foo .; cat foo
  install: ./foo: Bad address
  cat: foo: No such file or directory

This goes against the manual page, which states: "The install utility
attempts to prevent moving a file onto itself."

The check for the same inode number is only done when the full path is
given:

  $ echo foo > foo; install foo ./foo; cat foo
  install: foo and ./foo are the same file
  foo

The diff below attempts to handle the situation correctly if only the
directory name is given:

  $ echo foo > foo; install foo .; cat foo
  install: foo and ./foo are the same file
  foo

I'm not sure if the approach I'm using in the diff is correct. If a
directory name is given, I'm checking the inode for that directory
against the directory of the source, and exiting with an error if
they are the same. Would it be better to check if a file with the same
base name exists in the destination directory, and then check the inode
of that file against the file name?

If multiple from files are given, this will exit without processing all
of the files.  Is it better to just warn for each and change the exit
code at the end, so that all files are processed?  I can do that, but
I'm not sure which behavior is actually desired.  I think either
behavior is better than the current behavior of deleting the file.

This fixes an issue that causes the databases/ruby-ldap port to not
build if it loses a race.

Jeremy

Index: xinstall.c
===================================================================
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.51
diff -u -p -r1.51 xinstall.c
--- xinstall.c  11 Apr 2012 14:19:35 -0000      1.51
+++ xinstall.c  11 Sep 2012 23:09:03 -0000
@@ -40,6 +40,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <grp.h>
+#include <libgen.h>
 #include <paths.h>
 #include <pwd.h>
 #include <stdio.h>
@@ -167,8 +168,14 @@ main(int argc, char *argv[])
 
        no_target = stat(to_name = argv[argc - 1], &to_sb);
        if (!no_target && S_ISDIR(to_sb.st_mode)) {
-               for (; *argv != to_name; ++argv)
+               for (; *argv != to_name; ++argv) {
+                       if (stat(dirname(*argv), &from_sb))
+                               err(EX_OSERR, "%s", dirname(*argv));
+                       if (to_sb.st_dev == from_sb.st_dev &&
+                           to_sb.st_ino == from_sb.st_ino)
+                               errx(EX_USAGE, "%s and %s/%s are the same 
file", *argv, to_name, basename(*argv));
                        install(*argv, to_name, fset, iflags | DIRECTORY);
+               }
                exit(EX_OK);
                /* NOTREACHED */
        }

Reply via email to