Hacker News Reader

Comment by pgtan | Hacker News Reader

This bash code is a parade example how NOT to write scripts.

What would you change about it ? I'd be happy to learn how to write better code.

OK, for starter the while loop. Main rule of writing shell scripts is, Use The Shell, Luke. Don't start external programs, if your shell (which is already running) can do it. "while /usr/bin/true" is nonsense, every time an external program gets executed for nothing. There are plenty of bash (since this shell is used in the example) internals, which evaluate to true, like ":", "test 1", "(( 1 ))", maybe others too. So "while :" should be better. But why even use dummy true statement, since the loop terminates with read? You can put the read as argument to while directly: "while read -r line; do" ... Same with echo and date. Just use one single printf builtin.

while read -r line ; do printf '%(%FT%T%z)T %-9s %s' -1 "$(/usr/bin/id --user --name --real)" "$line" ; done

Thanks for the pointer about the useless use of true.

Using builtins may be a security risk as they can be overloaded (hence the use of full paths everywhere).

I did not know about bash's extension of printf to print a date. I need to use the builtin to use it though. But I've received good advice elsewhere on how to do that securely, so I'll do it and do a write-up because it's not information that's easy to come by.

Your advice is very helpful, thank you.

Bash and zsh both allow:

  function /usr/bin/cat { echo hello; }; 
  /usr/bin/cat
  hello

So specifying a path isn't as useful as it seems

for builtin security see for example https://unix.stackexchange.com/questions/102746/how-to-invok...

Reply on news.ycombinator.com