<font face="Default Sans Serif,Verdana,Arial,Helvetica,sans-serif" size="2"><br>On 06/23/2016 01:11AM Andrew Jeffery wrote:<br>><br>>On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:<br>>> From: Milton Miller <<a target="_blank" href="mailto:miltonm@us.ibm.com">miltonm@us.ibm.com</a>><br>>> <br>>> Notify the user if image wildcard expansion fails instead of<br>>printing<br>>> message about failing to find partition to flash.<br>>> <br>><br>>This kind of problem is why I have reservations about our early/late<br>>shell scripts. I wonder how many more of these issues are lurking...<br>><br>>> The update script errors with the message that it can't figure out<br>>what<br>>> partition to flash for /run/initramfs/image-* if there are no<br>>images<br>>> pending.<br>>> <br>>> Signed-off-by: Milton Miller <<a target="_blank" href="mailto:miltonm@us.ibm.com">miltonm@us.ibm.com</a>><br>>> ---<br>>>  .../obmc-phosphor-initfs/files/obmc-update.sh               | 13<br>>+++++++++++--<br>>>  1 file changed, 11 insertions(+), 2 deletions(-)<br>>> <br>>> diff --git<br>>a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob<br>>mc-update.sh<br>>b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob<br>>mc-update.sh<br>>> index 1dbf65f..7120a18 100755<br>>> ---<br>>a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob<br>>mc-update.sh<br>>> +++<br>>b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob<br>>mc-update.sh<br>>> @@ -99,6 +99,7 @@ checkmount=y<br>>>  whitelist=/run/initramfs/whitelist<br>>>  image=/run/initramfs/image-<br>>>  E="ERROR:"<br>>> +imglist=<br>>>  <br>>>  while test "$1" != "${1#-}"<br>>>  do<br>>> @@ -184,7 +185,15 @@ then<br>>>          fi<br>>>  fi<br>>>  <br>>> -for f in $image*<br>>> +imglist=$(echo $image*)<br>><br>>If there are no wildcard matches doesn't this expand to the string<br>>"/run/initramfs/image-*"? Why are we using echo in a subshell?<br>><br><br>Because that causes path expansion to be performed before<br>assigning the output to the parameter.<br><br>>> +if test "$imglist" = "$image*" -a ! -e "$imglist"<br>><br>>What's the idea behind the first condition ("imglist" = "$image*")?<br>>Won't they always be the same?<br>><br><br>No, because parameter splitting and path expansion are<br>suppressed by the double quotes here but path expansion<br>occurred in the echo above.<br><br>One could argue the image name * should not be used and<br>suppress the second condition.<br><br>>> +then<br>>> +        # shell didn't expand the wildcard, so no files exist<br>>> +        echo "No images found to update."<br>><br>>Is this worthy of the ERROR: prefix?<br>><br><br>I decided ERROR only gets used for error level to stderr,<br>while this is a warning or just a notice to stdout.<br><br>>> +        imglist=<br><br>If * is not expanded the imglist variable is cleared here.<br><br>>> +fi<br>>> +<br>>> +for f in $imglist<br>><br>>If imglist is empty don't we run into the same issue with findmtd,<br>>just<br>>that we have printed the warning above? Should we exit?<br>><br><br>No because it got cleared in the the clause above.  I am<br>adding this feature so in the future I don't have to touch<br>an image-name (creating an empty file and getting that<br>notice instead) to get to the restore and copy files phase<br>in a future update to init (see the clean-rwfs-filesystem<br>section there), but feel its more user-friendly overall.<br><br>Hope this helps,<br>milton<br></font><BR>