-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update scripts #32
base: master
Are you sure you want to change the base?
Update scripts #32
Conversation
fixes FPS problems in streams |
done | ||
if ls -l "/tmp/x$$" | grep root | ||
then | ||
echo 'install 9front |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should say nixos not 9front :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe he is using NixOS already, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah his repo would be wrapped in heaps of nix scripts if he did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some thoughts.
fi | ||
fi | ||
done | ||
if ls -l "/tmp/x$$" | grep root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is the long (-l
) form of ls(1), I have to wonder whether you're looking for a user or group called 'root', or a file called 'root'. Especially if it's the latter, you can just use glob filename pattern matching. There's almost never a need nor a recommendation to use ls(1) within a shell script; it's often unreliable and usually redundant.
Also, ls(1) will not be checking for whether it's a directory, a file, or something else (IE: FIFO or a block special file) which can cause problems when you want something specific.
Instead of using ls(1), you could opt to use find(1):
find /tmp -xdev -maxdepth 1 -type f -name "x$$" -user 'root' -o -group 'root' | ...
That's more reliable and robust, searching only for files, keeping to the same filesystem, being non-recursive (like your use of ls
), matching only files, and files whose user or group is 'root'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is quite obvious that I am trying to check whether the file that was written earlier is owned by user or group root. Sure, I could hassle with—I could even replace the whole if
command list by a single find
(see -exec
). Or, I could just ignore the existence of an abomination like find till there is a need for it. I could also use something like id
to check whether we are root. Many pretty much equivalent fine options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find(1) offers a lot of great functionality in an efficient and fairly portable way, as I demonstrated, but you're of course welcome to write in whatever way you please.
' | tee /etc/issue >/etc/motd | ||
fi | ||
rm "/tmp/x$$" | ||
} >/dev/null 2>/dev/null & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, this won't work properly. The correct* non-BASH version would be: >/dev/null 2>&1
To my knowledge, at least in BASH, redirection works in reverse; this is likely for Bourne Shell, too. So, in this case, STDERR is first redirected to STDOUT, then STDOUT and STDERR are both redirected to the desired location of '/dev/null'.
* Unless this changed at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you aware that I am not duplicating file descriptors anywhere but opening new ones? Do you see a problem with opening /dev/null
twice in the same process? I don’t.
Also, I don’t care about the Bourne Shell: The whole thing would not work with the Bourne Shell, since it doesn’t have comments. I care about the POSIX shell language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't care about Bourne Shell? You probably should, since that's the syntax of shell you're using.
Did you even read what I said? I feel like your needless hostility to everything I've said to try to help you is going to make any interaction with you a waste of time, so I'll leave you be. Thanks at least for replying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The syntax of shell I am using is the syntax POSIX specifies, which is not the Bourne Shell. The Bourne Shell is an ancestor of the POSIX command language, and not used anymore—at least I have yet to see someone using a port of the Bourne Shell to something like Linux or today’s BSDs.
The last line of the code I am adding is a comment. You know what the Bourne Shell says when stumbling upon a comment?
#: not found
Why exactly do you think >/dev/null 2>/dev/null
is a problem?
do | ||
if sed 1q "$i" | grep '^#![ ]*/bin/sh' | ||
then | ||
if ! grep '^# mark$' "$i" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a wildly unnecessary use of grep(1). Instead, and much more efficiently, make use of glob pattern matching within a case statement:
case $i in
\#\ mark)
COMMANDS ;;
esac
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you a troll or can you just not read shell scripts? $i
is the name of a file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is GitHub, not Reddit. You needn't resort to such petty acts as name-calling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I am sorry, please disregard the name-calling question.
@terminalforlife I suggest you take the code my PR adds (from the shebang to the |
useful changes