Hot answers tagged

9

do ... done is a compound command; every subcommand shares the file descriptors; so teeing the the loop has the same effect as teeing each subcommand. Two subsequent invocations of echo can be combined together. cat $file is a dreaded UUOC. A basename invocation can be avoided by changing directory to $FOLDER. ls is absolutely unnecessary. The shell already ...


8

Your functions shouldn't depend on variables but use specified parameters. You always specified variables before calling your function instead of just specifying them as parameters, e.g: DIR=Box1 Host=192.168.1.100 mnt but you can easily write it as mnt Box1 192.168.1.100 I would also only parse your command line arguments in one place and create a ...


8

Two points: echo $(something that already prints) is just redundant. This is simpler source ./buzz.sh $i There does not appear to be any need for two separate files. Put buzz in a function. Also, you are using a lot of subshells that are unnecessary (a). I'd stick with the more verbose but clearer if buzz() { if (( $1 % 15 == 0 )); then echo '...


7

If you're using source, you shouldn't have the shebang in the second script. If you're using the shebang, you shouldn't use source. But rather than use source directly, I would source the script once at startup, and in it define a function that was called at the later times. You should use {} instead of () to avoid forcing a separate subshell to spawn. I ...


7

Accessing environment variables in jq This doesn't really work as it is...: RESOURCE_NAME="example.com" # ... cat resource-changes.json | \ jq '.Changes[].ResourceRecordSet.Name=env.RESOURCE_NAME' | \ # ... jq cannot access RESOURCE_NAME if it's not exported. The fix is simple: export RESOURCE_NAME="example.com" Iterating over a list of values This is ...


7

fileExt="*.js" allFiles=$(find ./ -name $fileExt) This is a bug. The wildcard in $fileExt will be expanded by the shell, and cause a syntax error when the current directory has more than one matching file in it: $ touch a.js b.js $ fileExt="*.js" $ find ./ -name $fileExt find: paths must precede expression: `b.js' find: possible unquoted pattern after ...


6

2018/04/24 edit I missed an obvious bug: is_linux_mint is not implemented correctly. The results of a return on an empty output of a command substitution, with or without quotes, is implementation-defined: The value of the special parameter '?' shall be set to n, an unsigned decimal integer, or to the exit status of the last command executed if n is not ...


5

First thing, you could turn the check if the checkbashisms tool is installed into a guard clause, shaving off one level of indentation: if ! command -v checkbashisms >/dev/null 2>&1; then echo "‘checkbashisms’ is not available; it can be installed from the ‘devscripts’ package." exit 1 fi The remaining logic can be simplified a lot with ...


5

Using a string for MOUNTPOINTS means you can't have any mount points with spaces of glob characters in the names. I realize you likely can't use an array if you want real portability but that's a severe limitation that you might want to check for. I don't know what you can do (without using an array) that will allow directories like that to work but you can ...


5

Consider putting the following at the beginning of every script: set -e set -u The first makes your script abort as soon as the first command exits with an error. This ensures that your script doesn’t try to operate after it’s failed. The second yields an error whenever you try to access an empty variable. I’m not sure why you’re using arrays for dotFiles ...


5

Aside from running essentially the same find command three times, the main issue is that you run a separate basename instance for every single found file. If you are using GNU find (verify with find --version), you can get find to print the basenames directly: find . -name '*.js' -type f -printf '%f\n' On my system this is about 900 times faster than ...


5

General Run shellcheck on this script - almost all variable expansions are unquoted, but need to be quoted. That will also highlight the non-portable echo -e (prefer printf instead) and a dodgy use of $(( where $( ( would be safer. I recommend setting -u and -e shell options to help you catch more errors. Flexibility Instead of requiring users to change ...


4

I note three potential problems with your approach, both dealing with special characters in filenames, as well as one comment on the example usage. for with find Consider a directory structure where some paths have spaces in them, say: Source ├── Constants.h └── Main Loop.c (One could argue the wisdom of using spaces in paths, but your script should be ...


4

I have have an issue with the use of ls in dotFiles=$(ls -da .* | grep -Pxv '\.git|\.+') The problem is that ls is difficult to parse correctly. It doesn't handle spaces, newlines or non-ascii characters in a portable way. Also, it is often aliased by users in different ways. See http://mywiki.wooledge.org/ParsingLs for a comprehensive list of the ...


4

A few comments. You make many copies of the files but JFFS2 seems to support hard links, so I would use links instead (see man ln and the ln command in the example below). Also you copy all files into temporary directories and then copy them to the target directory. This is unnecessary - just copy one file to the target directory and then link that to the ...


4

First of all, there are so many pitfalls associated with /bin/sh programming that I prefer to write all except the most trivial scripts in a language like perl, python or even awk. I realize that availability is a concern, but all of those languages are pretty standard now. If you write the script in a better scripting language you can get rid of the ...


4

I see a number of things that may help you improve this program. I don't have a Windows machine handy, so these will all be Linux-based observations. For local installation, don't use sudo Right now, when the program is created for a local user, the use of sudo to copy it means it ends up owned by root. That means that although it's in the local user's ...


4

If you are using bash then rather than mixing [ and [[, you should stick with one or the other, preferrably [[. See What is the difference between test, [ and [[ ? You might also want to consider preferring printf over echo for a more robust approach. See this amazingly detailed analysis of why printf is better than echo.


4

As to the security of this: if [ -x /usr/local/libexec/path_helper ]; then eval `/usr/local/libexec/path_helper` fi This runs the path_helper script, which presumably outputs a string, and then the command represented by the output string is run in the local context. So this initially presumes you trust path_helper to run anything it wants. I don't ...


4

I'm not a shell expert, but I think I could help a bit. The most relevant tips I could give are: 1. Quote your expansions Unquoted expansions are the most common source of bugs and security issues in shell scripts. There are cases where it's safe to leave expansions unquoted (as in foo=$bar) but, unless extremely necessary, it's preferable to double-quote ...


4

Get rid of unnecessary nested and combined ifs While here it goes to no extreme, it's advisable to check for errors first, and to do that one by one - if possible, which is exactly this case. Do some extra checks Only in case of 0 arguments, the script should output usage message. This script does not support multiple arguments, so if given more than 1, ...


4

Some notes: This script is really very brittle. It'll break if any of the tools change in any significant way, if your disk layout changes (see below), or if the sequence of steps for whatever reason is not right in future versions of Arch Linux. It's a nice exercise to make sure you've understood the installation process, but in my experience these scripts ...


3

There are a number of things here which concern me. First up, the stderr log file does not have the time on the file name (missing _%T). This is a classic copy-paste+partial-fix issue, you copied the same code to multiple places, then needed to fix it, but you only fixed some of them. The solution to that is to extract the code to just one place, and reuse ...


3

The other answers made very good points, in the following I will mention only what was missed. First of all, don't be afraid to use Bash instead of /bin/sh. Bash must exist in any decent system today, and it will enable you to use arrays, which would simplify the handling of mount points with spaces. That said, for the record, there is a way to make the ...


3

There are a number of concerns I have here, some of them style related, but mostly about the actual functionality.... mounts /var and /home may not necessarily be actual mount points. You should first check to see whether it is a mount point before testing it. This is because it may inaccurately reflect the / space if it is not actually a mount. ...


3

Instead of this: if [ $(dpkg-query -W -f='${Status}' $i 2>/dev/null | grep -c "ok installed") -eq 0 ]; then A better way to write the same thing: if ! dpkg-query -W -f='${Status}' $i 2>/dev/null | grep -q "ok installed"; then Instead of this: if [ "$response" == "y" ] || [ "$response" == "Y" ]; then A simpler way to write is: ...


3

That's a four calls to cat /etc/redhat-release. A case statement could help reduce the clutter: case "`cat /etc/redhat-release`" in *"release 5."*) cp /etc/inittab /etc/inittab.install sed -i 's/ca::ctrlaltdel/#ca::ctrlaltdel/' /etc/inittab init q ;; *"release 6."*) echo 'exec false' > /etc/init/control-alt-delete.conf ;; *"...


3

I don't have enough reputation to comment, but I am rather intrigued by your script, because I also enjoy writing large programs in the shell, and recently have been trying write modular shell scripts. Perhaps the reason nobody has tried to answer your question is because the script is rather intimidating and somewhat confusing. At the risk of appearing ...


3

Don't write custom scripts for common sysadmin tasks. Using the proper tools will get the job done better, and the solution will be more maintainable — especially by any colleagues you might have. Using monit, for example, this would just be a matter of writing a simple configuration file. set logfile /root/process_monitor.log check process myproc with ...


3

You should always double-quote variables that are used as filesystem paths. This is the same script but with variables correctly double-quoted: # Parse through all show folders. for show in /Users/sanjeetsuhag/Desktop/*; do # Check if it is a folder. if [ -d "$show" ]; then # Parse through all season folders. for season in "$show"/*; ...


Only top voted, non community-wiki answers of a minimum length are eligible