install: Simplify path construction.

[des]

Summary

There's no need to copy the path twice to split it into base and dir.
We simply call basename() first, then handle the two trivial cases in
which it isn't safe to call dirname().

While here, add an early check that the destination is not an empty
string. This would always fail eventually, so it may as well fail
right away. Also add a test case for this shortcut.

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

[markj]

usr.bin/xinstall/xinstall.c

722

style(9) no longer requires local variables to be declared at the beginning of a function, and NetBSD has the declarations here, though the code's otherwise diverged somewhat.

744

basename(3) is documented as returning a pointer to "/" or "." in some degenerate cases, in which case this comparison doesn't make sense. Is it guaranteed that that won't happen here?

I also don't quite understand the comparison itself. It distinguishes between "//foo" and "/foo", is that intentional?

[des]

usr.bin/xinstall/xinstall.c

744

1. You're right, if to_name_copy is empty then the comparison is invalid. I'll add a check that prevents calling makelink() with an empty to_name. In all other cases, basename() will return a pointer to somewhere within its input.

2. Yes, because the latter is the only case in which dirname() would clobber the base name.

[des]

exit early if target is empty

[des]

usr.bin/xinstall/xinstall.c

722

Chalk it up to personal preference.

[markj]

usr.bin/xinstall/xinstall.c

744

Yes, because the latter is the only case in which dirname() would clobber the base name.

Sorry, I don't follow. dirname("//foo") and dirname("/foo") give the same result.

[des]

usr.bin/xinstall/xinstall.c

744

dirname("/foo") will overwrite the f in foo with a NUL, clobbering base, but dirname("//foo") will overwrite the second /, leaving base unscathed.

[markj]

usr.bin/xinstall/xinstall.c

744

I see, ok. It's not easy to understand this block; I'd add a comment explaining what it does.

[des]

usr.bin/xinstall/xinstall.c

744

It's a good think you asked for an explanatory comment, because it made me realize that I'd missed a fairly common case... I'll update the diff shortly.

[des]

fix cwd case, add comments and test case for empty destination

[des]

fix test