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