views:

497

answers:

4

The following DOS script snippet has a bug:

if not exist %MyFolder% (
    mkdir %MyFolder%
    if %errorlevel% GEQ 1 (
        rem WARNING: the line above has a bug! 
        rem %errorlevel% will be the errorlevel 
        rem of the if statement because of the (parentheses)
        echo Error: Could not create folder %MyFolder%
        goto AnErrorOccurred
    )
)

The fix is to use setlocal enabledelayedexpansion as follows:

setlocal enabledelayedexpansion
if not exist %MyFolder% (
    mkdir %MyFolder%
    if !errorlevel! GEQ 1 (
        rem WARNING: the line above has a bug! 
        rem !errorlevel! will be the errorlevel 
        rem of the if statement because of the (parentheses)
        echo Error: Could not create folder %MyFolder%
        endlocal & goto AnErrorOccurred
    )
)
endlocal

A full explanation of why is available here: http://stackoverflow.com/questions/877232/batch-file-fails-to-set-environment-variable-within-conditional-statement

I want to audit my code to find instances of this bug, I figure a Regex would be an appropriate match, but haven't managed to get one working...

I think the ingredients should be: Match an environment variable surrounded with %percentsigns% That is inside (parentheses)

Any suggestions?

A: 

You can't solve it with Regex, because you can't count the parenthesis with a regular language. For example:

Stuff (
More Stuff (
Less Stuff )
A %var%
Less Stuff )

There is no ( between the last ) and the variable. Since I can't count how many `( appear before to know if there's one open, I can't do this with regular expressions.

Daniel
A: 

As you can see, he is using ! rather than % even in the rootlevel of the batch. So basically you should be able to alternate every % to an ! for your environmental variables.

Hurix
+1  A: 

grepWin

I would use grepWin. Depending on the number of instances you have to find, you could write a regex that will give you all of them, plus some false positives.

Example: bug.bat

if not exist %MyVar% echo Hi!

if not exist %MyFolder% (
    mkdir %MyFolder%
    if %errorlevel% GEQ 1 (
        rem WARNING: the line above has a bug!
        rem %errorlevel% will be the errorlevel
        rem of the if statement because of the brackets
        echo Error: Could not create folder %MyFolder%
        goto AnErrorOccurred
    )
)

Then use a regular expression to match all lines that start with if and have an open parenthesis:

$ grep "^[ \t]*if.*(" bug.bat
if not exist %MyFolder% (
    if %errorlevel% GEQ 1 (

grepWin will show you all the files that match.

Percent Symbols

By request:

grep "^[ \t]*if.*%.*%.*(" bug.bat
Dave Jarvis
+1 Because I like the answer, I'll hold out a little while before accepting in case someone can cook up one that detects the %percentsigns% too.
Don Vince
BTW, the last parenthesis needs a backslash to escape it, instead of "^[ \t]*if.*(" it needs "^[ \t]*if.*\("
Don Vince
When I escape the last parenthesis, I see the following error: "grep: Unmatched ( or \(". The escape might be required for grepWin. Not for Unix grep, though.
Dave Jarvis
A: 

One other way is to use the AND (&&) and OR (||) tests for success (errorlevel 0) or error (errorlevel > 0), as in:

if not exist %MyFolder% mkdir %MyFolder%|| (
        echo Error: Could not create folder %MyFolder%
        goto AnErrorOccurred
)

I hope this can help.

And by the way, you do not fail to set the variable, you fail to read it back.

What happen is, within parenthesis, using %%'s give you the states which was before you entered the said parenthesis. The only thing I knows work well is the math "set /a toto+=1" which will correctly increment the variable. Otherwise you have two options:

Either use a called function to set the variable or use the setlocal ENABLEDELAYEDEXPANSION statement, as previously stated, and use !!'s within the parenthesis.

Jay