I wrote out a routine a couple of weeks back that queries a server for a list of users and then resets the user's connection based on how long their session has been active/idle.  The long form of this script repeated many of the same commands in a For loop that contained an IF statement.

I decided to move many of the repeated commands to a function to simplify/optimize the script but in doing so I separated the "IF" commands from the "For" loop and now my script isn't working properly.  Obviously putting the script back together the way it was will solve the problem but I am hoping that there is something to be learned here.  So, this is a sample of what I had:

Code: [Select]For /f "tokens=1-8 delims=,:/ " %%A in ('query user ^| findstr /i /v "disc USERNAME"') Do (
::Set variables using the data from the query above
set name=%%A
set status=%%D
set idletime=%%E

        If /i %%E GTR 5 (
    Reset Session %%C
    Echo !ncol!!scol!!icol!!time! >>"%SessionLog%"
)
)

^ That works just fine.  This is what I changed it to:

Code: [Select] Call :_fn_FORMATTABLE
If /i !IDLETIME! GTR 5 (
Reset Session !SESSIONID!
Echo !ncol!!scol!!icol!!time! >>"%SessionLog%"
)


:_fn_FORMATTABLE
For /f "tokens=1-8 delims=,:/ " %%A in ('query user ^| findstr /i /v "> USERNAME"') Do (
::Set variables using the data from the query above
Set NAME=%%A
Set SESSION=%%B
Set SESSIONID=%%C
Set DIDLETIME=%%E:%%F
Set IDLETIME=%%E
Set LOGONTIME=%%H

Goto:EOF
)

Again, when the IF condition was inside the For/Do loop everything worked.  Also, the script doesn't fail as it is written now but because the SESSIONID variable isn't being looped through (I think), it only holds the last value assigned to it by the query that's made.  In other words there may be 40 user sessions that meet the condition of the "IF" statement but only one session is processed.  Is there a way to call a function like my :_fn_FORMATTABLE without having to output all of the values found for SESSIONID to a txt file for the "IF" statement to run against? I guess what I'm asking is if there is a way to continue the for/do loop that is in the function as if it is written with the "IF" statement?

Thanks for any help you can give me.  I really don't want to have to revert to setting variables and processing the same for/do command  4 separate times in my script.  Note that both the function and the older version of the script include many other variables that I set for formatting output to a log.  So while the code I included doesn't look so bad, it is several lines longer in both versions of my script.  It's not so bad when it's all lumped together in a function but the script gets hard to navigate when it repeats time and again in the body of the script.

MJYou may get a better result by providing the FULL script and someone who has the time and interest can analyse it and provide an optimised version, assuming it is achievable.

A problem with asking a question about a small segment is often that it is out of context - and the best solution may be entirely different when the context is known.

Foxidrive +1
You could have code in your script that may be affecting the environment without realizing it.OK guys, I can't remember a time when you've let me down so the entire script is below.  This version of the script is the one that I want to make work (as opposed to the previous version where the contents of the :_fn_FORMATTABLE function was written out in each ":__clearsessions_" section of the script and everything worked .  My problem with this version of the script is with the call to :_fn_FORMATTABLE.

Code: [Select]Cls
Echo Off
:: Created by: MJ
:: 11/13/2014
:: Description: Contains the tools and routines used to update and reboot  servers
:: Dependencies:  Script must be called from the Server Maintenance Menu script
:: Command Window Settings
Set SCRIPTITLE=REBOOT/UPDATE UTILITY
TITLE %SCRIPTITLE%
Color 9F
mode con cols=60 lines=18 >nul

::---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
:ENVIRONMENT
::Allows special updates to be installed/made during the regular server maintenance routine
Set URGENT=YES
::Direct the script to execute the process (routines in this script) that was selected on the Server Maintenance Menu
Set UChoice=%1
::Modifies logic to allow new copies of the "DeleteProfiles" components to be downloaded to the server
Set DelProUp=YES

setlocal enabledelayedexpansion
Set STAR=****************************************************************************
Set ShSTAR=********************************************
Set Line=--------------------------------------------------------
Set ShLine=---------------------------------------------
Set DLine========================================================
Set LogDLine===========================================================================
Set ShDLine=============================================
Set "spaces=                               "
Set "indent=     *"
Set CPUPath=\\domain-01\dfs$\k****k\users\k****k_matt\homedir\Scripts\==Program Updaters==\Adobe Java Updates
Set RepositoryPath=\\domain-01\dfs$\k****k\groups\ITadmin\Script Repository
Set SessionLog=\\%computername%\c$\Util\AutoReboot\Logs\[Recent]\%date:~10,4%%date:~4,2%%date:~7,2%_Session.log
Set ProgDir=\\%computername%\c$\Util\AutoReboot
:ENDENVIRONMENT
:DEPCHECK

:END DEPCHECK
:Start
If [%UChoice%]==[] Color 4E & Echo %SCRIPTITLE% & Echo %STAR% & Echo. & Echo. & Echo This script must be called from the Server Maintenance Menu script^^! & Echo. & Echo. & Echo %STAR% & timeout /t 3 >Nul & Goto End
If %UChoice%==1 Set PROCESS=REBOOTUP
If %UChoice%==2 Set PROCESS=REBOOTUP
If %UChoice%==3 Set PROCESS=___PCUT_Configure
If %UChoice%==5 Set PROCESS=___updates_WINDOWSUPDATE
If %UChoice%==6 Set PROCESS=___updates_HP
If %UChoice%==8 Set PROCESS=___updates_URGENT
Echo %SCRIPTITLE% & Echo %DLine% & Echo.
Goto %PROCESS%
:ENDStart

:REBOOTUP
:_rebootup_CONFIG
::WLBS
Call :_fn_WLBS
::Create Directories for logs and program files
If Not Exist "%ProgDir%\Logs\[Recent]\" md "%ProgDir%\Logs\[Recent]\" >Nul 2>&1
If Not Exist "%ProgDir%\Logs\Drain\" md "%ProgDir%\Logs\Drain\" >Nul 2>&1
If Not Exist "%ProgDir%\Logs\Session\" md "%ProgDir%\Logs\Session\" >Nul 2>&1
If Not Exist "%ProgDir%\Logs\WU\" md "%ProgDir%\Logs\WU\" >Nul 2>&1
If Not Exist "%ProgDir%\Logs\Profiles\" md "%ProgDir%\Logs\Profiles\" >Nul 2>&1
::**TEMPORARY**Move the contents of C:\Temp that was previously  home of the Reboot Utility's logs and program dependencies to C:\Util\AutoReboot
Move /Y "C:\temp\ProfileCleanup\Logs\*" "%ProgDir%\Logs\Profiles\" >Nul 2>&1
If Exist "C:\temp\AutoReboot" RD /s /q "C:\temp\AutoReboot"
If Exist "C:\temp\ProfileCleanup" RD /s /q "C:\temp\ProfileCleanup"
If Exist "C:\temp\Auto Reboot\" Move /Y "C:\temp\Auto Reboot\*" "C:\util\AutoReboot\Logs\Profiles\" >Nul 2>&1
If Exist "C:\temp\Auto Reboot\" RD /s /q "C:\temp\Auto Reboot\"
::**TEMPORARY**Remove the Auto Reboot folder.  The space in the name of this folder was causing problems with the script so it was replaced with the AutoReboot folder
If Exist "C:\util\Auto Reboot\" RD /s /q "C:\util\Auto Reboot\"
::All logging done by the Reboot_Update Utility is saved in C:\Util\AutoReboot\Logs\[Recent].  This folder shows all of the logs that were collected the last time the corresponding scripts ran and logs were collected.  The following routine moves these files into folders specific to their function, making room for another Set of the most recent logs.
If Exist "%ProgDir%\Logs\[Recent]\*WU.log" Move  /Y "%ProgDir%\Logs\[Recent]\*WU.log" "%ProgDir%\Logs\WU\" >Nul 2>&1
If Exist "%ProgDir%\Logs\[Recent]\*_Profiles.log" Move /Y "%ProgDir%\Logs\[Recent]\*_Profiles.log" "%ProgDir%\Logs\Profiles\"  >Nul 2>&1
If Exist "%ProgDir%\Logs\[Recent]\*_Session.log" Move /Y "%ProgDir%\Logs\[Recent]\*_Session.log" "%ProgDir%\Logs\Session\" >Nul 2>&1
If Exist "%ProgDir%\Logs\[Recent]\*_Drain.log" Move /Y "%ProgDir%\Logs\[Recent]\*_Drain.log" "%ProgDir%\Logs\Drain\" >Nul 2>&1
::Create the Session Reset log
Echo SESSION RESET LOG    %date% >>"%SessionLog%"
Echo %LogDLine% >>"%SessionLog%"
Echo. >>"%SessionLog%"
Echo       USER                 STATUS     IDLE TIME     RESET TIMESTAMP   >>"%SessionLog%"
Echo ---------------------     --------   -----------   -----------------   >>"%SessionLog%"
:_rebootup_INITIATE
Set /P=* Initiating the server reboot routine....... <Nul
timeout /t 2 >Nul & Echo Done^^! & Echo.
:__rebootup_CLEARSESSIONS
Set /P=* Searching %computername% for logged in users.... <Nul
Call :_fn_QUERYUSERS CLEARSESSIONS
:___clearsessions_DISCONSOLE
::Query the server for disconnected sessions and reset them
start "" tsadmin.msc
Set /P=* Resetting disconnected sessions............ <Nul
Call :_fn_FORMATTABLE
If /i !SESSIONID!==Disc (
Reset Session !SESSION!
Echo !ncol!!dscol!!icolnul!!time! >>"%SessionLog%"
)
If /i !SESSION!==Console (
LOGOFF !SESSIONID!
Echo !ncol!!dscol!!icolnul!!time! >>"%SessionLog%"
)
Echo Done^^! & timeout /t 2 >Nul
Call :_fn_QUERYUSERS

:___clearsessions_ACTIVE
call :_fn_MESSAGE 10
Echo %indent%-4-* Minute Countdown....
timeout /t 240 >Nul
:____active_LONGIDLE
Set /P=* Resetting Long Idle sessions .............. <Nul
Call :_fn_FORMATTABLE
If /i !LOGONTIME! LSS 32 (
Reset Session !SESSIONID!
Echo !ncol!!scol!!dicol!!time! >>"%SessionLog%"
)
Echo Done^^! & timeout /t 2 >Nul
Call :_fn_QUERYUSERS

:____active_SHORTIDLE
call :_fn_MESSAGE 5
Echo %indent%-4-* Minute Countdown....
timeout /t 240 >Nul
Set /P=* Resetting Short Idle sessions ............. <Nul
Call :_fn_FORMATTABLE
If /i !IDLETIME! GTR 5 (
Reset Session !SESSIONID!
Echo !ncol!!scol!!icol!!time! >>"%SessionLog%"
)
Echo Done^^! & timeout /t 2 >Nul
Call :_fn_QUERYUSERS

:____active_LOGOFF
Call :_fn_MESSAGE 2
Echo %indent%-2-* Minute Countdown....
timeout /t 120 >Nul
Set /P=* Logging off remaining sessions ............ <Nul
Call :_fn_FORMATTABLE
Logoff !SESSIONID!
Echo !ncol!!scol!!icol!!time! >>"%SessionLog%"
Echo Done^^! & Echo. & timeout /t 2 >Nul

:__rebootup_clearsessions_END
::Next 2 lines complete the logs. 
Echo. >>"%SessionLog%" & Echo. >>"%SessionLog%"
Echo %ShDLine%  >>"%SessionLog%"
:__rebootup_UPDATES
Call :_fn_WLBS
:___updates_URGENT
Set /P=* Installing URGENT updates.................. <Nul
start /wait Call "%RepositoryPath%\Scripts - Terminal Server\Printers\RemovePrinterBloat.bat"
start /wait Call "\\domain-01\dfs$\k****k\groups\ITadmin\Script Repository\Scripts - Terminal Server\Hotfix\KB2479710 (Remote Desktop Services)\KB2479710_Installer.bat" called
Echo Done^^! & timeout /t 2 >NUL
If %UChoice%==8 Goto END
If %UChoice%==1 Goto __rebootup_PCUT
:___updates_CPU
::Update Flash, Shockwave, Java, Adobe Reader, GotoMeeting
Set /P=* Installing Program Updates.................<Nul & PING -n 2 localhost >Nul
Start /wait Call "%CPUPath%\CoreProgramsUpdater.bat"
Echo Done^^!
:___updates_WINDOWSUPDATE
mode con cols=60 lines=10 >nul
Set /P=* Installing Windows Updates.................<Nul & PING -n 2 localhost >Nul
CScript "\\%computername%\c$\util\AutoReboot\wsusAutoUpdate.vbs" >NUL
Echo Done^^! & timeout /t 2 >NUL
:___updates_HP
Set /P=* Starting HP system updates................. <Nul
IPConfig /all | Findstr /I "Virtual Hyper-V" >Nul
If %ErrorLevel%==1 ("C:\Program Files\Internet Explorer\iexplore.exe" "https://127.0.0.1:2381/cgi-bin/vcagent/cgi") Else (Echo ***V-NIC detected.*** & Echo Press any key to continue after Windows updates are finished applying. & pause >Nul)
Echo Done^^! & timeout /t 2 >NUL
If %UChoice%==6 Goto END
:__rebootup_updates_END
:__rebootup_PCUT
:___PCUT_Configure
Set /P=* Configuring Profile Cleanup Task........... <Nul
If Not Exist "%ProgDir%\Logs\Profiles\" md "%ProgDir%\Logs\Profiles\"
If /I [%DelProUp%]==[YES] (
Del "%ProgDir%\Del*" /Q >NUL 2>&1
Del "%ProgDir%\Sch*" /Q >NUL 2>&1
Copy "%~dp0\LocalServerFiles\*" "%ProgDir%" /Y >Nul 2>&1
) Else (
For /f %%I in ('dir /b "%~dp0\LocalServerFiles" ^| findstr /i /b "del sched"') do If Not Exist "%ProgDir%\%%I" Copy "%~dp0\LocalServerFiles\%%I" "%ProgDir%" >Nul
)
Echo Done^^! & timeout /t 2 >NUL
:___PCUT_Run
::Part of the Reboot/Update routine.  Run an unattended, silent version of the Profile Cleanup tool.
If %Uchoice%==3 (
Start Call "%ProgDir%\DeleteProfiles.bat"
Goto END
) ELSE (
Set /P=* Scheduling the Profile Cleanup Task........ <Nul
Start /wait /min Call "%ProgDir%\ScheduleCleanup.bat"
Echo Done^^! & PING -n 2 localhost >Nul
Goto ENDROUTINE
)
:_rebootup_pcut_END
:ENDROUTINE
Endlocal
Set /P=* Configuring k****k Shutdown................ <Nul
If %computername%==TS155-101 If %UChoice%==1 (C:\temp\k****kShutdown\k****kShutdown.exe -REBOOT -REASON "Scheduled Reboot" -NODIALOG -NOCONFIRM >Nul) Else (C:\temp\k****kShutdown\k****kShutdown.exe -REBOOT -REASON "Scheduled Reboot / Windows Updates / Program Updates" -NODIALOG -NOCONFIRM >Nul)
If %UChoice%==1 (\\domain-01.com\dfs$\util\k****kShutdown\k****kShutdown.exe -REBOOT -REASON "Scheduled Reboot" -NODIALOG -NOCONFIRM >Nul) Else (\\domain-01.com\dfs$\util\k****kShutdown\k****kShutdown.exe -REBOOT -REASON "Scheduled Reboot / Windows Updates / Program Updates" -NODIALOG -NOCONFIRM >Nul)
Echo Done^^!
Goto END

:FN
::Functions
:_fn_QUERYUSERS
::pause
query user | findstr /i /v "> USERNAME" >Nul 2>&1
Set fnErrorLevel=%ErrorLevel%
If [%1]==[CLEARSESSIONS] (
Echo Done^^! & timeout /t 2 >Nul
If %fnErrorLevel%==1 Echo %indent% Users not found, moving on * & Echo N^/A >>"%SessionLog%" & Goto __rebootup_clearsessions_END
) Else (
If %fnErrorLevel%==1 Echo %indent% Add'l users not found, moving on * & Goto __rebootup_clearsessions_END
)
Goto:EOF
:_fn_FORMATTABLE
::Query the server for "active" users but exclude the account that is logged in to run this script.  This account is signified by the > symbol next to their name in the output of the query user command.  If there are no active sessions then the script will move past the CLEARSESSIONS segment
For /f "tokens=1-8 delims=,:/ " %%A in ('query user ^| findstr /i /v "> USERNAME"') Do (
::Set variables using the data from the query above
Set NAME=%%A
Set SESSION=%%B
Set SESSIONID=%%C
::If %1=disc (Set DSTATUS=%%C) Else (Set SESSIONID=%%C)
Set STATUS=%%D
Set DIDLETIME=%%E:%%F
Set IDLETIME=%%E
Set LOGONTIME=%%H
::Manipulate the variables captured above to display the data in column form in logs
Set ncol=!NAME!!spaces!
Set ncol=!ncol:~0,27!
Set dscol=!DSTATUS!!spaces!
Set dscol=!scol:~0,12!
Set scol=!STATUS!!spaces!
Set scol=!scol:~0,15!
Set icolnul=    -!spaces!
Set icolnul=!icolnul:~0,16!
Set dicol=!DIDLETIME!!spaces!
Set icol=!IDLETIME!!spaces!
Set icol=!icol:~0,13!
Goto:EOF
)
:_fn_MESSAGE
If %1==10 Set "WARNTIME=10" & Set MESSAGE="**SERVER REBOOT**  The server you are logged into is scheduled to be rebooted in the next 10 minutes.  Please save your work and log out of k****k (Start>Log Off).  When you log back in you will be directed to a server that does not need maintenance."
If %1==5 Set "WARNTIME=05" & Set MESSAGE="**SERVER REBOOT**  The server you are logged into will be rebooted in 5 minutes. UNSAVED WORK WILL BE LOST!  Please log out of k****k (Start>Log Off) immediately."
If %1==2 Set "WARNTIME=02" & Set MESSAGE="**SERVER REBOOT**  The server you are logged into will be rebooted momentarily. UNSAVED WORK WILL BE LOST!  Please log out of k****k (Start>Log Off) immediately."
Set /P=* %WARNTIME% Minute Warning^^! - Notifying users......<Nul & timeout /t 3 >Nul
::Query sessions for all active users except the user who is running this script and alert them that the server will be rebooting.
For /f "tokens=1-8 delims=,:/ " %%A in ('query user ^| findstr /i /v "> disc USERNAME"') Do msg %%C %MESSAGE%
Echo Done^^!
Goto:EOF

:_fn_WLBS
::Drain 2003 servers that are managed by WLBS. 
If Exist C:\WINDOWS\System32\wlbs.exe (
Set /P=WLBS is draining %computername% for 60 minutes....<Nul
CD C:\
wlbs drainstop >Nul & wlbs drainstop >Nul
\\domain-01.com\dfs$\util\k****kShutdown\k****kShutdown.exe -StartMaintenance 60 -Reason "Scheduled Maintenance"
Echo Done^!
)
Goto:EOF
:END
::popd
Exit

I'm open to any suggestions for improvements in the script since I'm certain that there's still plenty for me to learn.  But I'm most concerned with somehow making my calls to _fn_FORMATTABLE work - preferably without having to output the results of this function to a text file that I can reference in the :__rebootup_CLEARSESSIONS section of the script.

Thanks for the help.

MJshouldn't this be using the environmental variable %username%?
Code: [Select]findstr /i /v "> USERNAME" Quote from: powlaz on February 17, 2015, 07:48:38 AM

OK guys, I can't remember a time when you've let me down so the entire script is below.  This version of the script is the one that I want to make work

OK guys, I can't remember a time when you've let me down so the entire script is below.  This version of the script is the one that I want to make work