Correct syntax for nested for loops in batch file

I am trying to query the registry value of an installed application on many servers. I could hardcode the servers in, and have a lot of repetition and it seems to work. But sometimes a list of servers may change, and I'd like to take a more sensible modular approach.

So I can provide a test file with a list of server names, and then query the registry values of all of these servers, outputting them in a logfile.

I think the issue is definitely with my loops/variable storing. Not sure exactly what is going on, but it seems like once I get one set of Registry values, these are outputted for every single server, regardless of their respective registry values.

Not too experience with this kind of scripting, so I have been sort of mashing this together through trial and error, below is what I have so far. Please point out any woeful syntax errors I've made.

So essentially this is to find a McAfee scan engine version installed to each server. The reporting from the McAfee console itself is not always reliable, and I want to check that the contents are successfully pulled down to each server.

There are two registry values required to create a full engine version, so I am required to pull both of these and then combine them into a single string.

At the moment I do get an output that looks correct, but it does not represent what is actually installed to each server. It is like one value is being picked up and then replicated.

On an additional note, this only seems to work when executed in command line. And the first 1-2 times it is ran, no values are pulled up. Get the feeling I am quite far away from a solution.

SET LOGFILE=C:\LogFile.log
For /f %%i in (servers.txt) DO (
for /f "tokens=3" %%a in ('reg query \\%%i\HKLM\SOFTWARE\McAfee\AVEngine /v EngineVersion32Major ^| find /i "EngineVersion32Major"') do set /a major=%%a
for /f "tokens=3" %%b in ('reg query \\%%i\HKLM\SOFTWARE\McAfee\AVEngine /v EngineVersion32Minor ^| find /i "EngineVersion32Minor"') do set /a minor=%%b
echo %%i: %major%.%minor% >> %LOGFILE%
)

Would expect an output like this:

server1 : 5900.5647
server2 : 6000.4589
server3 : 5900.5647
server4 : 5900.5647

Cheers for any help you can provide.

Solution

It has nothing to do with for syntax, but how cmd parses scripts. See How does the Windows Command Interpreter (CMD.EXE) parse scripts?

When cmd parses a block (anything within parens), processes it as a single line. So it expands any %var% to its actual content. Changes made to it, are not taken into account until the block is exited. To retrieve new content within a block, you must enable delayed expansion, which forces the parser to evaluate it for every iteration. Also, syntax must be changed from %var% to !var!

Here, also, removed the /a switch from set command, as you are not doing calculations, and you may get results you won't expect (imagine a minor version equal to 0080 that will be treated as octal (invalid) and would break the script). Also, note both var name and var content enclosed in quotes (set "minor=%%b"), to avoid undesired trailing/leading spaces.

More also, I think you don't need the ^| find /i "EngineVersion32Major part, as possibly the key named EngineVersion32Major will content only what you are looking for. And again, enclose data in quotes (never know when a space may appear). You may also change "tokens=3" by "skip=1 tokens=3" to avoid process the heading reg echoes.

Thus

@echo off
SetLocal EnableDelayedExpansion
SET LOGFILE=C:\LogFile.log
for /f %%i in (servers.txt) DO (
  for /f "tokens=3" %%a in ('reg query "\\%%i\HKLM\SOFTWARE\McAfee\AVEngine" /v "EngineVersion32Major"') do set "major=%%a"
  for /f "tokens=3" %%b in ('reg query "\\%%i\HKLM\SOFTWARE\McAfee\AVEngine" /v "EngineVersion32Minor"') do set "minor=%%b"
  echo %%i: !major!.!minor!>>%LOGFILE%
)
EndLocal

Also, this works within a block regardless delayed expansion is enabled or not (note double percent sign)

call echo %%i: %%major%%.%%minor%%>>%LOGFILE%

Another issue, any time a redirection is used within a block, the system opens and closes the file (or stream).

But

@echo off
SetLocal EnableDelayedExpansion
SET LOGFILE=C:\LogFile.log
>>"%LOGFILE%" (
  for /f %%i in (servers.txt) DO (
    for /f "tokens=3" %%a in ('reg query "\\%%i\HKLM\SOFTWARE\McAfee\AVEngine" /v "EngineVersion32Major"') do set "major=%%a"
    for /f "tokens=3" %%b in ('reg query "\\%%i\HKLM\SOFTWARE\McAfee\AVEngine" /v "EngineVersion32Minor"') do set "minor=%%b"
    echo %%i: !major!.!minor!
  )
)
EndLocal

Processes all commands in the block, so the file is opened and closed only once. This may improve performance, specially with large files.

BTW,

>>"%LOGFILE%" (
  ...
  ...
)

Is the same as

(
  ...
  ...
)>>"%LOGFILE%"