View Single Post
Old 06-14-2009   #5 (permalink)
Cary Shultz


 
 

Re: Look over this script.....could anything be done better?

Richard,

All excellent points......especially on the "even if ping works....". I
have run into corrupted WMI on specific machines several times! Always fun!

That will be revision 4 for these scripts......I need to learn how to do
that better than what I (think I) know how to do now....

BTW - that might be a good 'section' for your web site: error handling!

Thanks,

Cary


"Richard Mueller [MVP]" <rlmueller-nospam@xxxxxx> wrote in
message news:uF6fQTQ7JHA.1716@xxxxxx
Quote:

> Even when a computer responds to the ping, there is the chance that you
> will be unable to connect with WMI (although this is getting less likely).
> The remote computer might not have WMI installed, or WMI might be corrupt,
> or the class you are using might not be supported. In scripts where I ping
> before attempting to connect with WMI, I still trap the possible error.
> But I only use "On Error Resume Next" for the one statement that might
> raise the error. I test with Err.Number to see if an error was raised,
> handle the error, and restore normal error handling immediately. Otherwise
> bad things can happen. And if there is an unexpected problem, I always
> want to know.
>
> --
> Richard Mueller
> MVP Directory Services
> Hilltop Lab - http://www.rlmueller.net
> --
>
> "Cary Shultz" <cshultz@xxxxxx> wrote in message
> news:exv6$qP7JHA.1716@xxxxxx
Quote:

>> Pegasus~
>>
>> Thanks for the feedback and the tip (objTS.....). I learn something
>> everytime I post....you will be seeing more and more of me....
>>
>> Agreed on the On Error Resume Next. With this version of the script (one
>> more revision coming) I have kept that included because the script that
>> enumerates all of the servers (essentially queries AD and returns the
>> list of servers - without checking to see if there is actually a computer
>> associated with the object) has some servers that no longer exist
>> (working on cleaning up AD...using Joe's tools for that).
>>
>> Naturally, you know the next line from me.......without the "On Error
>> Resume Next" the script stops with the "object not found" at the
>> GetObject part of the script. In the testing environment that was not a
>> problem. When I tested in an environment that I built and maintain there
>> was no issue. However, when I ran it in a new client (where I have not
>> run Joe's tools yet) there were two objects for servers that do not exist
>> anymore.....STOP!!!!. So, for the moment I simply keep the "On Error
>> Resume Next". I hate it but for the time being it will stay...
>>
>> I just put in the "If (Err.Number <> 0) Then" part and that made the
>> output file so much nicer! So, that was my 2nd revision.
>>
>> The next thing is going to be adding comspec...ping.exe to actually ping
>> the servers as provided by that "input" file. Once I get that part down
>> then the "On Error Resume Next" part comes out! Good ridance!
>>
>> Thanks again,
>>
>> Cary
>>
>>
>> "Pegasus [MVP]" <news@xxxxxx> wrote in message
>> news:edcvL9O7JHA.3860@xxxxxx
Quote:

>>>
>>> "Cary Shultz" <cshultz@xxxxxx> wrote in message
>>> news:eUox20N7JHA.2656@xxxxxx
>>>> Good morning, All!
>>>>
>>>> Okay, I am learning VBScripting and have several scripts (about 15 or
>>>> so). Most of them look like the one below. I just would like to ask
>>>> those of your with far more experience than I have to take a look at
>>>> this and let me know if there is anything that is not correct or
>>>> anything that could be done better. We are running these scripts in
>>>> production environments (after some pretty stringent testing....). To
>>>> my knowledge there is nothing wrong with this script...it runs and
>>>> produces the desired results. I would just like to know if it can be
>>>> optimized. I have run it without the "On Error Resume Next" without
>>>> issue.....All of that was done during the testing.
>>>>
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>
>>>>
>>>> '==================================================================================================
>>>> '
>>>> ' VBScript Source File
>>>> '
>>>> ' NAME: #Fixed-Drives.VBS
>>>> ' VERSION: 1.0
>>>> ' AUTHOR: xxxxxxxxxx
>>>> ' COMPANY: xxxxxxxxxx
>>>> ' CREATE DATE : 06/03/2009
>>>> ' LAST MODIFIED : n/a
>>>> '==================================================================================================
>>>> ' COMMENT: This script will list all Drives on a Computer
>>>> '==================================================================================================
>>>>
>>>> Option Explicit
>>>> On Error Resume Next
>>>>
>>>> Dim objWMIService, colFixedDrives, objFD
>>>> Dim objFSO, objTextFile, objTS
>>>> Dim strComputer
>>>> Dim strInputFile, strOutputFile
>>>>
>>>> Const ForReading = 1
>>>> Const ForWriting = 2
>>>>
>>>> strInputFile = "C:\Scripts\Servers Info\Servers-TxtFile.txt" 'this
>>>> file list the servers against which this specific script is to
>>>> run....the servers are listed one line at a time
>>>> strOutputFile = "C:\Scripts\Results\Fixed-Drives.txt"
>>>>
>>>> Set objFSO = CreateObject("Scripting.FileSystemObject")
>>>> Set objTextFile = objFSO.OpenTextFile(strInputFile,ForReading)
>>>> Set objTS = objFSO.CreateTextFile (strOutputFile,ForWriting,False)
>>>>
>>>> Do Until objTextFile.AtEndOfStream
>>>> strcomputer = objTextFile.Readline
>>>>
>>>> Set objWMIService = GetObject("winmgmts:" _
>>>> & "{impersonationLevel=impersonate,authenticationLevel=Pkt}!\\"
>>>> & strComputer & "\root\cimv2")
>>>>
>>>> If (Err.Number <> 0) Then
>>>> objTS.WriteLine()
>>>> objTS.WriteLine "ERROR: Unable to connect to the WMI NameSpace on " &
>>>> strComputer
>>>> objTS.WriteLine()
>>>> objTS.WriteLine
>>>> "......................................................................"
>>>> Err.Clear
>>>> Else
>>>>
>>>> Set colFixedDrives = objWMIService.ExecQuery ("Select * from
>>>> Win32_LogicalDisk WHERE DRIVETYPE = 3",,48)
>>>>
>>>> For Each objFD in colFixedDrives
>>>> objTS.WriteLine "Computer Name: " & strComputer
>>>> objTS.WriteLine "Device ID: " & objFD.DeviceID
>>>> objTS.WriteLine "Drive Type: " & objFD.DriveType
>>>> objTS.WriteLine "Free Space: " &
>>>> ROUND(objFD.FreeSpace/1073741824,2) & " GB"
>>>> objTS.WriteLine "Size: " &
>>>> ROUND(objFD.Size/1073741824,2) & " GB"
>>>> objTS.WriteLine "Volume Name: " & objFD.VolumeName
>>>> objTS.WriteLine
>>>> "......................................................................"
>>>> Next
>>>>
>>>> End If
>>>>
>>>> LOOP
>>>>
>>>> objTextFile.Close
>>>> objTS.Close
>>>>
>>>> Set objTextFile = Nothing
>>>> Set objTS = Nothing
>>>> Set objFSO = Nothing
>>>>
>>>>
>>>>
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>
>>>>
>>>> Thanks everyone!
>>>
>>> Here are a few comments:
>>> - Having a global "on error resume next" is not a good idea because it
>>> makes trouble-shooting very difficult. It basically means "When you find
>>> an error, just pretend everything is OK and continue with whatever you
>>> were doing.". Much better to use the statement only when absolutely
>>> required and to turn it off immediately below the spot to be monitored.
>>> - The statement objTS.WriteLine "....................." could be written
>>> more elegantly as objTS.WriteLine string(30, ".").
>>>
>>> Other than this the script looks fine to me - keep up the good work!
>>>
>>
>
>
My System SpecsSystem Spec