Why is my if statement within a function causing an error in batch?

Question

I am coding a batch install script and I am trying to have this function run only if a certain variable is equal to 1. Whenever call this funtion I a normal syntax command("The syntax of the command is incorrect") followed by on the next line "C:\Windows\System32> break>" The code is below:

:updateStatus
IF %uploadInfo% EQU 1 (

    REM finds IP
    set ip_address_string="IP Address"
    set ip_address_string="IPv4 Address"
    for /f "usebackq tokens=2 delims=:" %%f in (`ipconfig ^| findstr /c:%ip_address_string%`) do (
        SET ip=%%f
        REM  goto :eof  
    )

    REM Removes spaces from IP
    SETLOCAL ENABLEDELAYEDEXPANSION
    for /f "tokens=* delims= " %%a in ("%ip%") do set ip=%%a
    for /l %%a in (1,1,100) do if "!ip:~-1!"==" " set ip=!ip:~0,-1!
    REM echo IP Adress: %ip%

    SET txtLoc=C:\CadVersionInfo\%ip%.txt
    SETLOCAL  DISABLEDELAYEDEXPANSION

    REM SETs time
    For /f "tokens=2-4 delims=/ " %%a in ('date /t') do (set mydate=%%a/%%b/%%c)
    For /f "tokens=1-2 delims=/:" %%a in ('time /t') do (set mytime=%%a:%%b)

    REM writes info
    if not exist C:\CadVersionInfo\ (mkdir C:\CadVersionInfo\)


    if exist "%txtLoc%" (
        del "%txtLoc%"
    )


    break>%txtLoc%


    @echo %trunk%.%build% >>"%txtLoc%"
    @echo %TA%>>"%txtLoc%"
    @echo %mdbname%>>"%txtLoc%"
    REM ~1 is status
    @echo %~1>>"%txtLoc%"
    @echo %mytime% %mydate%>>"%txtLoc%"

    @echo %notes%>>"%txtLoc%"

    REM Sets location on server to store data
    SET txtLocRemote=\\cd-ptt\CAD Downloads\Webserver\servers\*
    xcopy %txtLoc% "%txtLocRemote%" /q /y
 )
goto:eof

You need to enable and use [delayed expansion](http://stackoverflow.com/a/10558905), because you are modifying and reading variable values within a single line/block of code (note that `IF %uploadInfo% EQU 1 ( ... )` is treated as a single command line)... — aschipfl, Mar 01 '16 at 22:12
Only the top 10 lines are affected by that `IF %uploadInfo% EQU 1` test. Is the IP address already set if uploadinfo is NOT equal to 1 — foxidrive, Mar 01 '16 at 22:18
No, @foxidrive, the closing `)` is at the very bottom of the script before `goto:eof`; the `)` in the 10th line belongs to the `for /f` above... — aschipfl, Mar 02 '16 at 00:30
@aschipfl True indeed! Thanks. Your advice re `delayed expansion` is spot on, as variables are being set within the parentheses. Wes Larson's suggestion is also a good solution. — foxidrive, Mar 02 '16 at 00:49

score 2 · Answer 1 · answered Mar 02 '16 at 00:28

2

Rather than trying to deal with all of your variables using delayedexpansion, it's probably simpler to reverse your if statement and just use a goto, like this:

:updateStatus
IF NOT %uploadInfo% EQU 1 goto end

{do stuff}

:end

answered Mar 02 '16 at 00:28

Wes Larson

1,042
8
17

foxidrive · Answer 2 · 2016-03-02T01:18:23.307

Another solution is set the variables outside the loop regardless, and only process the loop if it is needed.

I have edited this post many times and in the end used the solution from Wes, adding this here just to illustrate some different ways to achieve parts of your code.

The spaces in the IP variable are removed in a simpler way.

The redirection to the file is changed - it can avoid trailing spaces using this method.

The * in SET "txtLocRemote=\\cd-ptt\CAD Downloads\Webserver\servers\*" is not legitimate syntax for xcopy and will need to be modified.

In this code block the two lines were replaced and the 2>nul simply hides irrelevant text from the console. It will always attempt to create the folder and delete the file, and no damage will be done if the folder already exists or if the file doesn't exist.

if not exist C:\CadVersionInfo\ (mkdir C:\CadVersionInfo\)
break>"%txtLoc%"

Here's your edited code:

:updateStatus

IF %uploadInfo% NEQ 1 goto :EOF

    REM finds IP
    set ip_address_string="IP Address"
    set ip_address_string="IPv4 Address"
    for /f "usebackq tokens=2 delims=:" %%f in (`ipconfig ^| findstr /c:%ip_address_string%`) do (
        SET ip=%%f
        REM  goto :eof  
    )

    REM Removes spaces from IP
        SET "ip=%ip: =%"
    REM echo IP Adress: %ip%

    SET "txtLoc=C:\CadVersionInfo\%ip%.txt"

    REM Sets location on server to store data
    SET "txtLocRemote=\\cd-ptt\CAD Downloads\Webserver\servers\*"

    REM SETs time
    For /f "tokens=2-4 delims=/ " %%a in ('date /t') do (set mydate=%%a/%%b/%%c)
    For /f "tokens=1-2 delims=/:" %%a in ('time /t') do (set mytime=%%a:%%b)


    REM writes info

    mkdir "C:\CadVersionInfo" 2>nul
    del "%txtLoc%" 2>nul


    >>"%txtLoc%" @echo %trunk%.%build%
    >>"%txtLoc%" @echo %TA%
    >>"%txtLoc%" @echo %mdbname%
    REM ~1 is status
    >>"%txtLoc%" @echo %~1
    >>"%txtLoc%" @echo %mytime% %mydate%

    >>"%txtLoc%" @echo %notes%

    xcopy "%txtLoc%" "%txtLocRemote%" /q /y

goto:eof

And there's still the unnecessary line `set ip_address_string="IP Address"`. This variable gets immediately overwritten on the very next line. This looks like it might be an attempt at compatibility with older Windows versions, but won't really do anything. — Wes Larson, Mar 02 '16 at 05:54

Why is my if statement within a function causing an error in batch?

2 Answers2