On 8/4/14, 21:33, "Kamble, Nitin A" <nitin.a.kam...@intel.com> wrote:
>
>On 8/4/2014 9:38 AM, Hart, Darren wrote:
>> On 7/29/14, 11:34, "Kamble, Nitin A" <nitin.a.kam...@intel.com> wrote:
...
>>
>>> + if [ -n "${INITRD}" ]; then
>>> + rm -f $dest/initrd
>>> + for fs in ${INITRD}
>>> + do
>>> + if [ -n "${fs}" ] && [ -s "${fs}" ]; then
>>
>> The -n test is unnecessary here. How would "for fs in ${INITRD}" result
>>in
>> an fs of "" ?
>The -n test is needed here, it checks whether the file exist or not.
Nope, -n tests if the string length is non-zero. See the bash manual
section "CONDITIONAL EXPRESSIONS". -s tests if the file exists and has a
size > 0.
>
>>
>>> + cat ${fs} >> $dest/ignited
>>> + fi
>> Some kind of a warning at least is warranted if a file appears in the
>> INITRD list but is either 0-size or non-existent.
>
>I tried to keep the original style of the code. But it makes sense to
>add a warning or even an error here.
Style is fine, but error checking is a functional thing. If it was missing
before, it was a bug.
>>>
>>> build_iso() {
>>> # Only create an ISO if we have an INITRD and NOISO was not set
>>> - if [ -z "${INITRD}" ] || [ ! -s "${INITRD}" ] || [ "${NOISO}" = "1"
>>>];
>>> then
>>> + if [ -z "${INITRD}" ] || [ "${NOISO}" = "1" ]; then
>>> bbnote "ISO image will not be created."
>>> return
>>> Fi
>> Perhaps the -s test should be replaced with a -s of $dest/initrd?
>The -s test is replaced by the loop few lines below.
>>> + # ${INITRD} is a list of multiple filesystem images
>>> + for fs in ${INITRD}
>>> + do
>>> + if [ ! -s "${fs}" ]; then
>>> + bbnote "ISO image will not be created. ${fs} is
>>> invalid."
>>> + return
>>> + fi
>>> + done
>> This additional loop could be eliminated by including this test above.
>> Right? Or am I missing something subtle here?
>That approach will leave a hole where, the function will continue when
>one of the filesystem image is invalid.
>So the loop is better here as it does not leave any hole.
But you've already built it right? So you have already tested for -s ${fs}
previously. The only thing that matters now is that the assembled image is
valid. $dest/initrd. Right?
--
Darren Hart Open Source Technology Center
darren.h...@intel.com Intel Corporation
--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
