# Tag Info

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

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 ...

7

Your code seems pretty much fine, and chicks has pointed out most of the issues I have. It's not a bad implementation of the approach you've chosen, but I have to wonder if that's the right approach. HTML is a complex format, and while using regex to parse it can work as long as you've structured it in a consistent manner known in advance (which definitely ...

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 ...

6

Good great variable names, but I quibble over a couple later on nice use of blank lines to organize code appropriate use of sed. (Regex search and replace is the only time to use it.) I wouldn't expect any XML tools to be an improvement. :) you're using a subset of bash strict mode using env for portability Questions Some things I wish the code had ...

6

One thing you might choose to do, given the neat correspondence of variable names and element ids, is to generate the sed script from the list of variables: # Produce a sed script that replaces TD elements' contents with value of # same-named variable. # Arguments: list of variable names make_subst_script() { for i do printf 's/\(<td id=&...

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

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

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

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

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 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

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

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 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 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 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

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