[PATCH] Fixed buggy [[ "$DEV" =~ "/dev/loop*" ]] tests with new isdevloop() function

[Marc Herbert]

This was accidentally working in old bash versions, not anymore in
Fedora 13's bash 4.1.7. Now using proper prefix matching instead of
the previous regex matching disguised as pattern matching.


---
 tools/livecd-iso-to-disk.sh |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/livecd-iso-to-disk.sh b/tools/livecd-iso-to-disk.sh
index f4d0855..846fa73 100755
--- a/tools/livecd-iso-to-disk.sh
+++ b/tools/livecd-iso-to-disk.sh
@@ -39,10 +39,15 @@ exitclean() {
     exit 1
 }
 
+isdevloop()
+{
+    [ x"${1#/dev/loop}" != x"$1" ]
+}
+
 getdisk() {
     DEV=$1
 
-    if [[ "$DEV" =~ "/dev/loop*" ]]; then
+    if isdevloop "$DEV"; then
        device="$DEV"
        return
     fi
@@ -65,7 +70,7 @@ getdisk() {
 }
 
 resetMBR() {
-    if [[ "$DEV" =~ "/dev/loop*" ]]; then
+    if isdevloop "$DEV"; then
        return
     fi
     getdisk $1
@@ -94,7 +99,7 @@ resetMBR() {
 }
 
 checkMBR() {
-    if [[ "$DEV" =~ "/dev/loop*" ]]; then
+    if isdevloop "$DEV"; then
        return 0
     fi
     getdisk $1
@@ -124,7 +129,7 @@ checkPartActive() {
     if [ "$dev" = "$device" ]; then
 	return
     fi
-    if [[ "$dev" =~ "/dev/loop*" ]]; then
+    if isdevloop "$dev"; then
         return
     fi
 
-- 
1.7.2

[Marc Herbert]

Le 03/08/2010 17:12, Marc Herbert a écrit :
>  This was accidentally working in old bash versions, not anymore in
>  Fedora 13's bash 4.1.7. Now using proper prefix matching instead of
>  the previous regex matching disguised as pattern matching. 
This broke due to a documented change in bash. More details here:

  http://thread.gmane.org/gmane.comp.shells.bash.bugs/15080

Or better: do not bother reading and avoid advanced, non-portable
and evolving bash features when basic features are enough (= apply my
patch).


Note: even without considering this change in bash, the previous code was
confusing patterns and regular expressions and working by chance.

[Bruno Wolff III]

On Wed, Aug 04, 2010 at 11:58:36 +0100,
  Marc Herbert <Marc.Herbert(a)gmail.com&gt; wrote:
>  Le 03/08/2010 17:12, Marc Herbert a écrit :
>  > This was accidentally working in old bash versions, not anymore in
>  > Fedora 13's bash 4.1.7. Now using proper prefix matching instead of
>  > the previous regex matching disguised as pattern matching.
>  
>  This broke due to a documented change in bash. More details here:
>  
>    http://thread.gmane.org/gmane.comp.shells.bash.bugs/15080
>  
>  Or better: do not bother reading and avoid advanced, non-portable
>  and evolving bash features when basic features are enough (= apply my
>  patch). 
The patch has been committed upstream.

[Marc Herbert]

> >> This was accidentally working in old bash versions, not
> anymore in
> >> Fedora 13's bash 4.1.7. Now using proper prefix matching instead of
> >> the previous regex matching disguised as pattern matching. 
> > This broke due to a documented change in bash. More details
> here:
> >
> >   http://thread.gmane.org/gmane.comp.shells.bash.bugs/15080 
>  The patch has been committed upstream. 
Bruno,

  Only the first hunk of this patch has been committed, the last three
other hunks (with the same bug) have not.

There was also a fifth hunk added by Jasper here:
   http://article.gmane.org/gmane.linux.redhat.fedora.livecd/3842
This fifth hunk is not committed either.

[Bruno Wolff III]

On Mon, Sep 13, 2010 at 14:13:53 +0100,
  Marc Herbert <Marc.Herbert(a)gmail.com&gt; wrote:
>  >>> This was accidentally working in old bash versions, not
> anymore in
>  >>> Fedora 13's bash 4.1.7. Now using proper prefix matching instead of
>  >>> the previous regex matching disguised as pattern matching.
>  
>  >> This broke due to a documented change in bash. More details here:
>  >>
>  >>   http://thread.gmane.org/gmane.comp.shells.bash.bugs/15080
>  
>  > The patch has been committed upstream.
>  
>  Bruno,
>  
>    Only the first hunk of this patch has been committed, the last three
>  other hunks (with the same bug) have not.
>  
>  There was also a fifth hunk added by Jasper here:
>     http://article.gmane.org/gmane.linux.redhat.fedora.livecd/3842
>  This fifth hunk is not committed either. 
Thanks. I'll look at this tonight.

[Bruno Wolff III]

On Mon, Sep 13, 2010 at 14:13:53 +0100,
  Marc Herbert <Marc.Herbert(a)gmail.com&gt; wrote:
>  
>  Bruno,
>  
>    Only the first hunk of this patch has been committed, the last three
>  other hunks (with the same bug) have not.
>  
>  There was also a fifth hunk added by Jasper here:
>     http://article.gmane.org/gmane.linux.redhat.fedora.livecd/3842
>  This fifth hunk is not committed either. 
I just confirmed that I had already picked up Jasper's patch from the
mail thread independently of that bug report.

Next it's back to the bug report to compare what I have to what I should
have.

[Bruno Wolff III]

On Mon, Sep 13, 2010 at 20:16:14 -0500,
  Bruno Wolff III <bruno(a)wolff.to&gt; wrote:
>  On Mon, Sep 13, 2010 at 14:13:53 +0100,
>    Marc Herbert <Marc.Herbert(a)gmail.com&gt; wrote:
>  > 
>  > Bruno,
>  > 
>  >   Only the first hunk of this patch has been committed, the last three
>  > other hunks (with the same bug) have not.
>  > 
>  > There was also a fifth hunk added by Jasper here:
>  >    http://article.gmane.org/gmane.linux.redhat.fedora.livecd/3842
>  > This fifth hunk is not committed either.
>  
>  I just confirmed that I had already picked up Jasper's patch from the
>  mail thread independently of that bug report.
>  
>  Next it's back to the bug report to compare what I have to what I should
>  have. 
I ended up finding the parts that were missing. And they should make it into
a build for beta.

[Marc Herbert]

Le 14/09/2010 04:04, Bruno Wolff III a écrit :
>  
>  I ended up finding the parts that were missing. 
Except for one part still missing...

At this time, git://git.fedorahosted.org/git/livecd is still missing
one hunk from the original patch, more precisely this one-line change:

 checkMBR() {
-    if [[ "$DEV" =~ "/dev/loop*" ]]; then
+    if isdevloop "$DEV"; then

[Bruno Wolff III]

On Mon, Sep 13, 2010 at 14:13:53 +0100,
  Marc Herbert <Marc.Herbert(a)gmail.com&gt; wrote:
>  >>> This was accidentally working in old bash versions, not
> anymore in
>  >>> Fedora 13's bash 4.1.7. Now using proper prefix matching instead of
>  >>> the previous regex matching disguised as pattern matching.
>  
>  >> This broke due to a documented change in bash. More details here:
>  >>
>  >>   http://thread.gmane.org/gmane.comp.shells.bash.bugs/15080
>  
>  > The patch has been committed upstream.
>  
>  Bruno,
>  
>    Only the first hunk of this patch has been committed, the last three
>  other hunks (with the same bug) have not. 
Please be more explicit. The original message I worked from only had
one patch in it.

I am short on time and don't have time to track down ambiguous stuff.

Unless this is a beta blocker, most likely it isn't going to get any more
work before the beta release. If I got a diff with the missing parts,
I might get a chance to look at it, otherwise I need to spend time on
other things.

They'll probably be a chance to get some more minor bug fixes in before
F14 final.

[Marc Herbert]

Le 14/09/2010 02:42, Bruno Wolff III a écrit :
>  Please be more explicit. The original message I worked from only had
>  one patch in it.
>  
>  I am short on time and don't have time to track down ambiguous stuff. 
Sorry Bruno but I have no idea where you got this truncated patch
from. I have just carefully looked again at a couple of email archives
and they all have:
- a four hunks patch (mine), plus
- a later five hunks patch (Jasper)

My knowledge of the modified script is unfortunately too limited for me
to comment on the last and fifth hunk added by Jasper.

Cheers,

Marc

PS: If you had only one hunk in your mailbox, then you maybe you should
get worried about some of the software you are using (I am serious).

[Bruno Wolff III]

On Tue, Sep 14, 2010 at 09:37:07 +0100,
  Marc Herbert <Marc.Herbert(a)gmail.com&gt; wrote:
>  Marc
>  
>  PS: If you had only one hunk in your mailbox, then you maybe you should
>  get worried about some of the software you are using (I am serious). 
I am not sure what happened. I had a few patches fail to apply because of
other patches and maybe this was one of them. I tried to make sure I
fixed those but could have made a mistake.

The other possibility is that I didn't realize that the patch was attached
and thought it was part of the message body (as multiple text parts are
displayed together) and botched copying it.

I thought I had picked up the one you mentioned, but must have made an
oops again. Thanks for double checking and point out the location.
I'll get this one in.