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
