On Thu, 2021-03-18 at 20:05 -0400, Scott Andrews wrote: > On Thu, 18 Mar 2021 22:17:07 +0100 > Pierre Labastie <pierre.labas...@neuf.fr> wrote: > > > On Thu, 2021-03-18 at 14:32 -0400, Scott Andrews wrote: > > > On Thu, 18 Mar 2021 11:51:09 -0500 > > > Bruce Dubbs <bruce.du...@gmail.com> wrote: > > > > > > > On 3/18/21 11:21 AM, Scott Andrews wrote: > > > > > > > > > > I am presently looking at and working on the LFS boot scripts. > > > > > They are in my opinion very rough state. > > > > > > > > > > I am going to clean them up and use the following format for > > > > > all of the individual scripts that will be used on my systems > > > > > as follows: > > > > > > > > > > Shebang line: #!/bin/bash > > > > > Comment Title block: the purpose of the script > > > > > Global variables: defined here > > > > > Local variables: not defined in functions > > > > > Source scripts: all outside scripts sourced here > > > > > Functions: all functions defined here > > > > > Mainline: main body > > > > > Cleanup: any cleanup that needs to be done > > > > > Exit: script terminates > > > > > > > > > > I will add comments to explain the goal/purpose of the script > > > > > and also what each function does. I am going to rewrite some > > > > > of the scripts so they will be self sufficient as possible. > > > > > > > > > > I will add ipv6 support using a service file much like > > > > > ivp4-static. This should allow ipv6 to be used on both dual > > > > > stacked systems and system that are ipv4 or ipv6 only. > > > > > > > > > > I am also going to build to test them before placing them > > > > > onto my working servers. > > > > > > > > > > Is there any interest by LFS on doing this or am I just > > > > > wasting my time posting here? > > > > > > > > What you propose seems to be mostly documentation, but other > > > > changes may be appropriate. I suggest you do a couple and let > > > > us review them. Then, with constructive feedback, the rest of > > > > the LFS scripts. > > > > > > > > I'll note though that you are the only one giving feedback and > > > > the scripts have only had minor changes since at least 2011. > > > > > > example follows......... > > > > Some shell style comments: > > > > > > remove SCRIPT_STAT from init-functions > > > > > > Correctly write check_script_status in rc > > > > > > function check_script_status { > > > local script=${1} > > It's preferable to use quotes around any variable you do not know > > in advance: script="$1" > > > > > SCRIPT_STAT="0" > > But here it is not needed: SCRIPT_STAT=0 > > Actually it is very much needed, as the variable is only used in the > rc script and it is only used as a "status". You are mis reading the > intent.
I meant the double quote are not needed... > > > > > > if [ ! -f ${script} ]; then > > Same here. Moreover, if script is empty, you'll get an error: > > if [ ! -f "${script}" ]; then > > > > > log_warning_msg "${script} is not a valid file." > > > SCRIPT_STAT=retval="1" > > =1. Note that "echo $SCRIPT_STAT" would print "retval=1", and that > > retval is empty after this statement. Is it what you want? > > Shouldn't there be an exit or return here? If $script is not a > > file, there is little chance it is executable. > > See my response to bruce > > > > > > fi > > > if [ ! -x ${script} ]; then > > if [ ! -x "${script}" ]; then ... > > > > > log_warning "${script} is not executable." > > > SCRIPT_STAT=retval="1" > > =1, and same note as above. > > > fi > > > return > > > } > > > > > > # Check script for file and executable > > > check_script_status > > > if [ "1" = ${SCRIPT_STAT} ]; then continue; fi > > You'd better quote ${SCRIPT_STAT} instead of 1. And, as said above, > > SCRIPT_STAT may contain "retval=1". > > > > I could point out the lack of quoting in the existing boot scripts > but I found that to not be necessary. Well, in that case, there is (almost) no chance that SCRIPT_STATE is empty, so quoting is not necessary. But quoting variables (except in very specific cases) is a good habit. I agree that quoting is inconsistent in lfs boot scripts. > > > > > > > > > That fixes the issue but I would rewrite check_script_status to > > > return true or false then then do this > > > > > > function check_script_status { > > > local script=${1} > > > local retval="0" > > > if [ ! -f ${script} ]; then > > > log_warning_msg "${script} is not a valid file." > > > retval="1" > > > fi > > > if [ ! -x ${script} ]; then > > > log_warning "${script} is not executable." > > > retval="1" > > > fi > > > [ "0" = ${retval} ] && return 0 || return 1 > > Why not `return retval'? > > > > > } > > > > > > if [ check_script_status ]; then continue; fi > > No brackets here. Brackets introduce predicates like -f, -x, or > > (in)equality. Testing the return of a command (a function here) is > > just done with `if <command>'. Note that `if [ check_script_status > > = 0 ]' wouldn't work either. > > > > Also, it seems to me that the logic would be `if ! > > check_script_status; then continue; fi' > > So when you are going to cleanup the boot scripts as they use > > if [ something ]; then whatever; fi > > and > > if something; then whatever; fi > Using brackets depends on "something". If something is a predicate, you need brackets. If something is a (compound) command, no brackets. I've not looked in details at the "if" in the boot scripts, but they must be correct in this respect, since they seem to work... -- http://lists.linuxfromscratch.org/listinfo/lfs-support FAQ: http://www.linuxfromscratch.org/blfs/faq.html Unsubscribe: See the above information page Do not top post on this list. A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? http://en.wikipedia.org/wiki/Posting_style