jcnelson / vdev

A device-file manager for *nix
GNU General Public License v3.0
101 stars 13 forks source link

Directory with the name "-n bus" being created. #44

Closed onimatrix81 closed 9 years ago

onimatrix81 commented 9 years ago

Looks like that one should be called just "bus". I worked around this by removing the -n switch from the echo line in usb-name.sh

jcnelson commented 9 years ago

@onimatrix81 Which echo are you using? The usb-name.sh helper uses echo -n to ensure that a newline isn't printed. This is because vdevd uses the exact string written to stdout to determine the path to create (but I could be persuaded to simply have vdevd strip trailing newlines, if this poses a practical problem).

onimatrix81 commented 9 years ago

I don't understand the question which echo? I don't know about my echo version, standard linux one? Anyways, removing the -n switch took care of that problem, but there's another one: the bus files have a "?" character appended to the end, such as: arpikolo vdev # ls vdev-test/bus/usb/002/ 001? 002? 004? and it isn't the usb-name.sh script that is doing it as I hacked the script to remove the last character echoed and ended up with a file "/bus/usb/002/00?" instead of "/bus/usb/002/001?"

onimatrix81 commented 9 years ago

I have no idea how the -n switch pushes through to the filename in usb-name.sh line 12: echo -n "bus/usb/$VDEV_OS_BUSNUM/$VDEV_OS_DEVNUM" but it does on my system. As a matter of fact I edited it to echo -jeepotato "bus/usb/$VDEV_OS_BUSNUM/$VDEV_OS_DEVNUM" and ended up with a vdev-test/-jeepotato bus/

jcnelson commented 9 years ago

@onimatrix81 The usb-helper.sh script uses /bin/dash as its shell, which means that the echo it should be using is the one built into dash (see dash(1) to confirm). However, looks like this isn't the case for you, since the echo built into dash honors the -n flag, but echo that is actually getting run on your machine does not.

There are several echo implementations you could be using, and not all of them honor the -n flag.

The ? you see in the path names is the newline character \n. Using echo -n should ensure that the newline character does not get printed. However, the echo program you are using does not honor the -n flag, so by removing -n, the string it prints won't be 001, but 001\n (which ls shows as 001?).

somasis commented 9 years ago

You should probably be using printf instead of echo. https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo

jcnelson commented 9 years ago

@Somasis This is a good point, but the program printf is usually installed to /usr/bin, and printf is not guaranteed to be a shell built-in. Vdev's helper scripts should only rely programs installed to /bin, /sbin, and /lib/vdev in order to support mounting /usr late in the boot process. While printf-the-dash-built-in meets this requirement, any reliance on dash-isms is considered a bug (as discussed in #42, vdev should not depend exclusively on dash, or any shell).

onimatrix81 commented 9 years ago

Indeed. My /bin/dash is dash is not a symlink though, so why wouldn't the -n work? It works fine with /bin/echo which probably is from coreutils. This is dash-0.5.8.2 on Gentoo for future reference. For now I got it fixed by replacing echo -n with printf only other "echo -n" was in subr.sh and i replaced that too. Testing shall continue now :)

somasis commented 9 years ago

printf is a built-in in just about any shell used nowadays. The only ones it isn't built-in in are some pdksh derivatives.

jcnelson commented 9 years ago

@Somasis Understood, but I'm interested in supporting pdksh in particular, since I hope to port vdev to OpenBSD at some point.

Given this discussion, I think the best way to solve this particular problem would be to have vdevd strip trailing newlines from paths its helpers create. I can't think of a real-world use-case where a path name would need to have \n in it, let alone have it at the very end.

jvvv commented 9 years ago

I checked OpenBSD's man page for printf; it states posix compliance, so that should not present an issue with using it there. I know that dash's implementation of printf follows posix and I'm faily certain that bash does as well. Using echo anywhere you want portability, though, will definitely lead to problems.

fbt commented 9 years ago

Actually POSIX does not include -n for echo: http://pubs.opengroup.org/onlinepubs/009604599/utilities/echo.html

It is not possible to use echo portably across all POSIX systems unless both -n (as the first argument) and escape sequences are omitted.

So if you want to use the echo builtin portably, you should use it without arguments.

It should be fine to use printf, as POSIX does include it: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html

jcnelson commented 9 years ago

@jvvv Expecting echo to honor -n everywhere was a mistake on my part, but as @fbt points out, echo with no arguments is POSIX-compliant. There are only two places in the code where I need echo -n, so for the time being, I'm just going to have vdev ship with a small helper binary called echo_n that implements echo -n. I've added it in b7ed48f020abab067728994692af47fee70e27ad (and speaking of POSIX compliance, I just got done removing all instances of sed -r in bfa2a63cfdbb476e99db8a12a3735a50c6786508).

I would be perfectly fine with using printf in place of echo and echo_n if printf was guaranteed to be available on the root partition (i.e. in /bin or /sbin). This is not the case for OpenBSD--its default shell does not have printf built in, and it is installed to /usr/bin by default.

EDIT: Moreover, printf is not guaranteed to be a shell built-in in some of the Linux distros that are interested in vdev, which puts them in the same boat as OpenBSD.

jvvv commented 9 years ago

I should have checked OpenBSD's path to printf. Keep up the good work. I will try to start testing in the near future, if work obligations don't get in the way.

jcnelson commented 9 years ago

@onimatrix81 Are you still seeing this bug on your end?

onimatrix81 commented 9 years ago

No. You figured it out way back (the echo -n thing). Maybe I should start closing my bugs when they become irrelevant =). On the other hand the talk might have something that interests others so maybe better you do that.

jcnelson commented 9 years ago

@onimatrix81 The conversation thread never goes away :). It just gets moved into the "closed" set of issues.