Solved

Why is this batch file looping?

Posted on 2009-07-12
11
345 Views
Last Modified: 2012-05-07
Please help:
I only wan this action to happen once but it echos Hi mutiple times then movesback to then next line in the firs file. I know I need to do something w/ and end statement or :eof but i cannot get themin the right place.


for /f %%A in (.\comp.txt) do set path=%%A & call :step1
pause
end

:Step1
for /f %%B in (.\user.txt) do set dir=%%B & call :step2

:step2
echo Hi
0
Comment
Question by:dpousson
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 6
  • 3
  • 2
11 Comments
 
LVL 43

Expert Comment

by:Steve Knight
ID: 24835959
You need a goto :eof at the end of your subroutine and before the start of any subs, i.e.

for /f %%A in (.\comp.txt) do set path=%%A & call :step1
pause
REM This is the end of the batch file, goto the end of file now
goto :eof

:Step1
for /f %%B in (.\user.txt) do set dir=%%B & call :step2
REM this is the end of Step1 sub
goto :eof

:step2
echo Hi
REM this is the end of the step 2 sub -- or it would drop to the end of the batch and work anyway
goto :eof

Not sure what you are trying to do specifically with the for loops but ask if not sure on any of it.

hth

Steve
0
 

Author Comment

by:dpousson
ID: 24836132
thanks that is very close to what I think I need but it is still executing the commands in step 2 or :IQ2 2 times before it continues to the next line in the first file. That first script was just a simple example....here is the actual script:

set serverpath=D:\Users\Home
set folder=ArchiveMail
set pcprofile=c$\Documents and Settings
set pst=Local Settings\Application Data\Microsoft\Outlook\*.pst
set NK2=Application Data\Microsoft\Outlook\*.NK2
set DesktopPST=Desktop\*.pst
set MyDocsPST=My Documents\*.pst

FOR /F %%A IN (.\compnames.txt) DO set pcpath=%%A& Call :IQ1
pause
goto :eof


:IQ1
FOR /F %%B IN (.\users.txt) DO set sharepath=%%B& Call :IQ2
goto :eof
:IQ2
xcopy /y /v "\\%pcpath%\%pcprofile%\%sharepath%\%pst%" "%serverpath%\%sharepath%\%folder%\"
xcopy /y /v "\\%pcpath%\%pcprofile%\%sharepath%\%NK2%" "%serverpath%\%sharepath%\%folder%\"
xcopy /y /v "\\%pcpath%\%pcprofile%\%sharepath%\%DesktopPST%" "%serverpath%\%sharepath%\%folder%\"
xcopy /y /v "\\%pcpath%\%pcprofile%\%sharepath%\%MyDocsPST%" "%serverpath%\%sharepath%\%folder%\"

goto :eof
0
 
LVL 16

Expert Comment

by:t0t0
ID: 24836333
There are a couple of things I ought to point out to you..

Firstly, you're setting the variable 'path'. If you are CERTAIN this is what you intend to do then fine however, you should be aware PATH is a system-wide environment variable and is best left alone. For this reason, it might be better to use PTH instead as in the code below.

Secondly, merely ECHOing 'Hi' is a pretty poor way to test your batch file. For this reason I have changed this to: 'ECHO %pth% %dir%' at least this way we can see what's happening with the variables too - and not just the logical flow of the batch file.

Thirdly, there is much contention over the use of 'GOTO :EOF' as opposed to 'EXIT /B'. It may help you to read the following article: http://www.robvanderwoude.com/exit.php

I prefer to use 'EXIT /B' as it enable me to return an ERRORLEVEL value.

In you code, you must end each sub-routine otherwise program control runs on to the next line of code - and quite possibly into another sub-routine as in your case.

Finally, the 'PAUSE' has no effect whatsoever other than to pause exiting the actual batch file. In your code, the 'PAUSE' will NOT pause inbetween iterations of the first FOR loop because it lies outside of the FOR's body.

To pause  iterations of the first FOR loop you would need to do the following:

   for /f %%A in (.\comp.txt) do (
      set pth=%%A
      call :step1
      pause
   )
   exit /b

or, if you're hell-bent on using compound statements then the following is the equivilent:

   for /f %%A in (.\comp.txt) do set pth=%%A & call :step1 & pause
   exit /b

The modified code below now runs correctly:

   @echo off
   for /f %%A in (.\comp.txt) do set pth=%%A & call :step1 & pause
   exit /b

   :Step1
   for /f %%B in (.\user.txt) do set dir=%%B & call :step2
   exit /b

   :step2
   echo %pth% %dir%
   exit /b

To close this discussion, be warned about using compound statements. Their behaviour can sometimes be undesirable. An example of this is in the following two lines of code. Try to predict what the output will be, then try it out for yourself:

   set name=dpousson
   set name=t0t0 & echo %name%
0
Forrester Webinar: xMatters Delivers 261% ROI

Guest speaker Dean Davison, Forrester Principal Consultant, explains how a Fortune 500 communication company using xMatters found these results: Achieved a 261% ROI, Experienced $753,280 in net present value benefits over 3 years and Reduced MTTR by 91% for tier 1 incidents.

 
LVL 16

Expert Comment

by:t0t0
ID: 24836448
See the correct method to do what you're trying to do:

@ECHO OFF
SET ServerPath=D:\Users\Home
SET Folder=ArchiveMail
SET PcProfile=c$\Documents and Settings
SET Pst=Local Settings\Application Data\Microsoft\Outlook\*.pst
SET NK2=Application Data\Microsoft\Outlook\*.NK2
SET DesktopPST=Desktop\*.pst
SET MyDocsPST=My Documents\*.pst

FOR /F %%a IN (.\CompNames.txt) DO (
   FOR /F %%b IN (.\Users.txt) DO (
      XCOPY /Y /V "\\%%a\%PcProfile%\%%b\%Pst%" "%ServerPath%\%%b\%Folder%\"
      XCOPY /Y /V "\\%%a\%PcProfile%\%%b\%NK2%" "%ServerPath%\%%b\%Folder%\"
      XCOPY /Y /V "\\%%a\%PcProfile%\%%b\%DesktopPST%" "%ServerPath%\%%b\%Folder%\"
      XCOPY /Y /V "\\%%a\%PcProfile%\%%b\%MyDocsPST%" "%ServerPath%\%%b\%Folder%\"
   )
)
PAUSE
0
 

Author Comment

by:dpousson
ID: 24836887
very very nice, thank you so much. I learned a ton form u as you can tell I am a beginner and just do things the only way I know how.

This still does not quite work right. It is mixing the files instead of keeping coorelating lines of the 2 .txt files together. I wan tit to pull files from a supplied pc name from the users profile then put it on in the destination directory then go the next pc and username and keep it all separte.

Thanks again
0
 

Author Comment

by:dpousson
ID: 24837027
here is an example to test. I zipped up my files. it is copingy file1 and file2 to user1 and user2, but I want it to copy file1 to user1 and file2 to user2 andd not both to both

rename Copytest.cmd.txt to Copytest.cmd
thanks
Dev.zip
0
 
LVL 43

Expert Comment

by:Steve Knight
ID: 24837734
<<See the correct method to do what you're trying to do:>>

There is NO correct way to do programming.  A correct way is one that works and is within the experience of the person writing it.  There are better ways or worse ways but if it works, it works.

Anyway you have some nice solutions there so will leave you with t0t0.

Steve
0
 
LVL 16

Expert Comment

by:t0t0
ID: 24838480
dragon-it

The 'correct' method (above) refers to the nesting of FOR loops to achieve what the asker was attempting to do using CALLs. I wasn't implying it's the 'correct' solution to the asker's question.



0
 
LVL 16

Accepted Solution

by:
t0t0 earned 500 total points
ID: 24838557
I ran the code from your zip file. Below is the results. Please state if this is not what you want, and if possible please state (given the BEFORE state) how you want the AFTER state to look like.
   BEFORE     |
              |----DOCS
              |     |----DIR1
              |     |     '----FILE1.TXT
              |     '----DIR2
              |           '----FILE2.TXT
              '----HOME
 
 
 
   AFTER      |
              |----DOCS
              |     |----DIR1
              |     |     '----FILE1.TXT
              |     '----DIR2
              |           '----FILE2.TXT
              '----HOME
                    |----USER1
                    |     |----FILE1.TXT
                    |     '----FILE2.TXT
                    '----USER2
                          |----FILE1.TXT
                          '----FILE2.TXT

Open in new window

0
 
LVL 16

Expert Comment

by:t0t0
ID: 25277318
Please tend to this question,

Thank you.
0
 
LVL 16

Expert Comment

by:t0t0
ID: 25654183
thank you
0

Featured Post

Announcing the Most Valuable Experts of 2016

MVEs are more concerned with the satisfaction of those they help than with the considerable points they can earn. They are the types of people you feel privileged to call colleagues. Join us in honoring this amazing group of Experts.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

AutoHotkey is an excellent, free, open source programming/scripting language for Windows. It started out as a keyboard/mouse macros product, but has expanded into a robust language. This article provides an introduction to it, with links to addition…
Introduction: Recently, I got a requirement to zip all files individually with batch file script in Windows OS. I don't know much about scripting, but I searched Google and found a lot of examples and websites to complete my task. Finally, I was ab…
Email security requires an ever evolving service that stays up to date with counter-evolving threats. The Email Laundry perform Research and Development to ensure their email security service evolves faster than cyber criminals. We apply our Threat…
Are you ready to implement Active Directory best practices without reading 300+ pages? You're in luck. In this webinar hosted by Skyport Systems, you gain insight into Microsoft's latest comprehensive guide, with tips on the best and easiest way…

756 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question