[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [BusyBox] [PATCH] sed 'T' command
From:       Rob Landley <rob () landley ! net>
Date:       2005-05-20 19:01:31
Message-ID: 200505201501.31587.rob () landley ! net
[Download RAW message or body]

Darn it.

On Friday 20 May 2005 10:48 am, Rob Landley wrote:
> On Wednesday 18 May 2005 01:44 am, Rob Landley wrote:
> > On Tuesday 12 April 2005 01:41 pm, Colin Watson wrote:
> > > This patch implements the 'T' command in sed. This is a GNU extension,
> > > but one of the udev hotplug scripts uses it, so I need it in busybox
> > > anyway.
> > >
> > > Includes a test; 'svn add
> > > testsuite/sed/sed-branch-conditional-inverted' after applying.
> >
> > Applied, however I'm a bit concerned about the logic.
>
> I vill not buy thees logic, it ees scratched.
...
>Despite this, a patch is attached

that sucked as badly as the original problem, just in a different way.  (This 
time screwing up the 't' test...)

I'm not awake enough to come up with a variant that only has just one zeroing 
of substituted.  In the case of the first match finding a substitution did 
occur, we want to zero substituted before we fall through to the test for T 
so that T finds one _didn't_ occur and we fall through again to the branch.  
But if we jump straight into T and it finds there was a substitution (thus 
breaking instead of jumping), we have to zero it before the actual break 
because the logic is "did a substitution occur since last test", and we just 
tested (even though the test was false, it still selected which of two code 
paths to go down and counts as a test).

So to get both of these right, we need a zero before the 'T' test and another 
zero if 'T' triggers.  Sigh.

Less sucky patch.  (Still sleep deprived, but possibly Wright this time.  One 
of the brothers, at least...)

Rob

["walrus2.patch" (text/x-diff)]

--- busybox.bak/editors/sed.c	2005-05-18 21:11:30.000000000 -0400
+++ busybox/editors/sed.c	2005-05-20 14:50:52.000000000 -0400
@@ -999,15 +999,17 @@
 						}
 						break;
 					}
-
 					/* Test/branch if substitution occurred */
 					case 't':
-						if(!substituted) break;
+						if (!substituted) break;
 						substituted=0;
 						/* Fall through */
 					/* Test/branch if substitution didn't occur */
 					case 'T':
-						if (substituted) break;
+							if (substituted) {
+								substituted=0;
+								break;
+							}
 						/* Fall through */
 					/* Branch to label */
 					case 'b':


_______________________________________________
busybox mailing list
busybox@mail.busybox.net
http://codepoet.org/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]