Two For Loops Using %1 - Delayed Expansion?

Question

My working batch file scans a long list of remote servers, copies anything there to a local server, checks the log file for a keyword, and if the keyword is found sends an email. I noticed it is always sending emails, even with a blank log file.

I discovered both FOR loops are using the %1 variable for their output - as seen in ECHO %1 and each line of the called :servermove. For lack of a better explanation it is not resetting %1 to null between loops.

I reviewed almost a dozen SO posts and am somewhat confident using SETLOCAL ENABLEDELAYEDEXPANSION would resolve this. That is where my understanding ends and I am unsuccessful thus far.

Here is the relevant code:

SET DATE=%date:~4,2%-%date:~7,2%-%date:~10,4%
SET HH=%time:~0,2%
SET MN=%time:~3,2%
SET TSTAMP=Time Run is %HH%%MN%
SET DATETIME=%DATE% at %HH%%MN%
SET LOGFILE="\\nt980a3\CreditFileImagesTransmission\LogFiles\%DATETIME%-File Move Log.txt"

SET MailDst=
SET MailSrc=
SET MailSrcName=Center to LDSD File Mover
SET OKMailSub=A Branch Has Sent You Some Files

ECHO %DATETIME% > %LOGFILE%
ECHO. >> %LOGFILE%

FOR /F "tokens=1" %%A IN (%~dp0SourceServers.txt) DO CALL :ServerMove %%A

:cleanuplogs
PUSHD "\\nt980a3\CreditFileImagesTransmission\LogFiles" &&(
FORFILES /S /M *.txt /D -45 /C "CMD /C DEL /Q @path"
) & POPD

:mailtest
FOR /F "tokens=*" %%A IN (%LOGFILE%) DO CALL :searchlog "%%A"

:searchlog
ECHO %1 | find "\\nt">NUL
IF NOT ERRORLEVEL 1 GOTO successmail
GOTO exit

:successmail
IF EXIST %temp%\to.txt DEL %temp%\to.txt
FOR %%a IN (%MailDst%) DO ECHO %%a>>%temp%\to.txt
"%~dp0sendmail.exe" /TO=%temp%\to.txt /FROM=%MailSrcName% ^<%MailSrc%^> /REF=%OKMailSub% /MESSAGE=%LOGFILE% /HOST=

:exit
EXIT

:ServerMove
DIR /S /B \\%1\CreditFileImagesTransmission\*.* >> %LOGFILE%
XCOPY /E /C /I /Y "\\%1\CreditFileImagesTransmission\*.*" "\\nt980a3\CreditFileImagesTransmission\%DATE%\%HH%%MN%\"
FOR /D %%P IN ("\\%1\CreditFileImagesTransmission\*.*") DO RMDIR "%%P" /Q /S
DEL /Q /S "\\%1\CreditFileImagesTransmission\*.*"

I tried changing :mailtest to use %%B in both instances but that also fails. Placing SETLOCAL ENABLEDELAYEDEXPANSION and its counterpart ENDLOCAL before one or the other loop and changing the %%A to !A! does not work either.

Would someone kindly point out the error in my ways and offer suggestions or resources that will help me resolve this?

Answer 1

%1 is the first parameter provided to the procedure - either from the command-line (in the main procedure) or the parameter following the procedure name in call :procedurename parameter1.

In your case, %1 to :servermove is an entry from SourceServers.txt and %1 to :searchlog is each line from %LOGFILE%.

Since you've censored your batch, what you've posted makes little sense. For instance, the :searchlogs routine will take the first line from %LOGFILE% and go to successmail or cleanlogs depending on whether that first line contains the target string \\nt. What it does from there, we can't tell.

We're faced with an XY problem - trying to fix a solution, not a problem.

---

First problem: Don't use date as a user-variable. It's a "magic variable" which contains the date but it's overridden by a specific set statement.

Having run :servermove for each entry in SourceServers.txt, you are - accumulating a directory list from \CreditFileImagesTransmission\*.* on that server. - copying those files to server nt980a3 with a date/timestamp but not including the source-servername so any duplicate name anywhere will overwrite an earlier version. I suggest you include %1 into your destination-name. - deleting subdirectories - deleting files.

I'd suggest you simply remove the directory \\%1\CreditFileImagesTransmission\, then re-create it.

I'd also suggest that you add an extra line

goto :eof

after the del /q /s... line. This will cause execution to be transferred to the end-of-file (the colon in :eof is required) and may seem superfluous, but it ensures that the routine has a defined endpoint - if you add a further routine, there is no way the :servermove routine will continue into your new code.

After each server has been processed, you proceed to the :cleanuplogs routine, which I presume deletes logs older than 45 days.

Your next statement is a real problem. What it will do is grab the very first line of the logfile (which contains "%DATE% at %HH%%MN%" with the date resolved as you've set at the start and it then processes this line in :searchlog; there is no \\nt in this line, so errorlevel is set to 1, and the batch proceeds to :EXIT (not a good label in my view, since it's a keyword); executes an exitand should terminate the batch.

This appears not to be what it is actually doing, and I'm at a loss to explain why.

I'd suggest changing

:mailtest
FOR /F "tokens=*" %%A IN (%LOGFILE%) DO CALL :searchlog "%%A"

:searchlog
ECHO %1 | find "\\nt">NUL
IF NOT ERRORLEVEL 1 GOTO successmail
GOTO exit

to

:mailtest
find "\\nt" %LOGFILE%>NUL
IF NOT ERRORLEVEL 1 GOTO successmail
:failmail
echo "\\nt" was found in the log
pause
GOTO exit

but I can't test that...

Answer 2

:mailtest
FOR /F "tokens=*" %%A IN (%LOGFILE%) DO CALL :searchlog "%%A"

You are missing a GOTO :EOF or similar goto here because it will drop through to the routine below once the above is finished.

:searchlog
ECHO %1 | find "\\nt">NUL
IF NOT ERRORLEVEL 1 GOTO successmail
GOTO exit

Answer 3

I feel you can not carry the %1 of first for loop to others. Try to transfer that to another variable like below.

:ServerMove

set servername=%1

DIR /S /B \\%servername%\CreditFileImagesTransmission\*.* >> %LOGFILE%
XCOPY /E /C /I /Y "\\%servername%\CreditFileImagesTransmission\*.*"     "\\nt980a3\CreditFileImagesTransmission\%DATE%\%HH%%MN%\"
FOR /D %%P IN ("\\%servername%\CreditFileImagesTransmission\*.*") DO RMDIR "%%P" /Q /S
DEL /Q /S "\\%servername%\CreditFileImagesTransmission\*.*"

Cheers, G