On 05/17/2011 11:34 PM, Bryan Kadzban wrote:
> Few nits related to the shell. In the network script:
>
>> # Process individual configuration files
>> for file in `ls "${dir}"`; do
> Ew. :-) How about:
>
> for file in "${dir}"/* ; do
> ONBOOT=`grep "ONBOOT" "${file}" | sed ...
> ...
>
>
I had used it that way for debugging to create a list. There was a
reason that it was left that way, but I can't for the life of me recall
what it was, and it obviously wasn't important.
> (since it always does a ${dir}/${file} as written)
>
> In the ifup/ifdown scripts:
>
>> else
>> grep "${INTERFACE}" /proc/net/dev 2>&1> /dev/null
>> if [ "${?}" != "0" ]; then
> Why not a (simpler):
>
> if grep "${INTERFACE}" /proc/net/dev 2>&1> /dev/null ; then
>
> instead? (This is done in several places: anywhere the script is
> testing the value of "$?" can potentially be simplified.)
>
Simply preference, it looks more explicit to my eyes if someone wants to
review the script and learn from it. I went ahead and changed all four
examples to inline tests.
>> echo "ERROR: ${INTERFACE} is not a valid network interface."
>> echo ""
>> exit2
> That should be "exit 2", right? :-)
Fixed.
>> fi
>> fi
Thanks for the review.
-- DJ Lucas
--
This message has been scanned for viruses and
dangerous content, and is believed to be clean.
--
http://linuxfromscratch.org/mailman/listinfo/lfs-dev
FAQ: http://www.linuxfromscratch.org/faq/
Unsubscribe: See the above information page
