[RESOLVED] Folder Looping Problem

Hello, I'm working on a new version of my old SecureMD5 program (which you may have seen on download.com or softpedia). One of the new features is Recursive Generation.

The program loops through each file in a folder and generates a checksum for it. Then it places the checksums in a file.

The only problem is... nothing happens. I click Generate, and the programs just sits there and does nothing. Here's my code for the Generate button:

VB Code:

Try
                    Dim dir As New System.IO.DirectoryInfo(TextBox7.Text)
                    Dim md5 As New System.Security.Cryptography.MD5CryptoServiceProvider
                    Dim hash As Array
                    Dim i As Integer
                    Dim files As Array
                    Dim sb As New System.Text.StringBuilder
                    files = dir.GetFiles
                    For i = files.Length To files.GetUpperBound(0)
                        Dim f As New System.IO.FileStream(i.ToString, IO.FileMode.Open)
                        md5.ComputeHash(f)
                        hash = md5.Hash
                        For Each hashByte As Byte In hash
                            sb.Append(String.Format("{0:X1}", hashByte))
                        Next
                        System.IO.File.AppendAllText(TextBox8.Text, sb.ToString)
                        f.Close()
                    Next
                Catch ex As Exception
                    MessageBox.Show("An error occured while trying to generate a checksum!" & Environment.NewLine & ex.Message, "Error!", MessageBoxButtons.OK, MessageBoxIcon.Error)
                End Try

TextBox7 is the textbox for the folder path and TextBox8 is the textbox for the output file (with the checksums). Just thought you might need to know that.

VB Code:

For i = files.Length To files.GetUpperBound(0)

presumably should be

VB Code:

For i = 0 To files.GetUpperBound(0)

If you'd used a breakpoint and stepped through your code I'm sure you'd have seen this pretty quickly. Also, whenever you are using the length of an array or collection to control a For loop I STRONGLY suggest including the "Step 1" on the end. If you don't and the array or collection is empty then the loop will try to access the first element/item and throw an exception, because it will implicitly try to iterate from zero to -1 with a step of -1.

Thank you for replying, but when I change it to 0, I get

Quote:

---

Can not find file "C:\Documents and Settings\My Documents\0

---

Also, I dunno how to use breakpoints. I never really took the time to learn how to use the built in debug functions. :(

Dim f As New System.IO.FileStream(i.ToString, IO.FileMode.Open)

This is the problem. You are casting the counter variable to a string.

Should be files(i).Fullname

Bill

Quote:

---

Originally Posted by conipto

Dim f As New System.IO.FileStream(i.ToString, IO.FileMode.Open)

This is the problem. You are casting the counter variable to a string.

Should be files(i).Fullname

Bill

---

Actually just files(i) as it is a String array rather than a FileInfo array.

DataHunter2009, you ABSOLUTELY MUST learn how to use the debugger. It's one of the main reasons IDEs exist. Do you really think it's fair to ask us to spend the time to fix your problems when you already have the tools at your disposal to do it yourself? That's a gentle reprimand but a reprimand nonetheless. :) Using a breakpoint and the Watch window would have fixed your problem in about 1 minute.

Also, given that you don't really need the loop index I'd tend to use a For Each loop rather than a For loop. That would get rid of both the issues pointed out in one go:

VB Code:

For Each fileName As String In files
    Dim f As New System.IO.FileStream(fileName, IO.FileMode.Open)

Doesn't DirectoryInfo.GetFiles return a FileInfo Array?

Oh and dude, those untyped Array's are ugly :P

Bill

Quote:

---

Originally Posted by conipto

Doesn't DirectoryInfo.GetFiles return a FileInfo Array?

Oh and dude, those untyped Array's are ugly :P

Bill

---

Oops, quite right you are. :blush: I guess I must have just assumed without actually having looked. *self-flagellates* It actually should just be a String array though as no other member of the FileInfo is used:

VB Code:

For Each filePath As String In IO.Directory.GetFiles(folderPath)

Thanks for the answers! I changed the for statment and it worked great! :D

Now the only problem is that when it does the next file, it sticks on the hash of the file before it. For example, I hashed a file and it gave me: "915083DB5073C6FCC9A6834D5D286B", but the next file gave me: "915083DB5073C6FCC9A6834D5D286B438C5BE9B4E66824201B873AC7DDB". Any ideas?

VB Code:

System.IO.File.AppendAllText(TextBox8.Text, "MD5 Hashes Generated by IntegriCheck" & Environment.NewLine & "Folder: " & TextBox7.Text & Environment.NewLine)
                    Dim hash As Array
                    Dim sb As New System.Text.StringBuilder
                    For Each filePath As String In IO.Directory.GetFiles(TextBox7.Text)
                        Dim f As New System.IO.FileStream(filePath, IO.FileMode.Open)
                        Dim md5 As New System.Security.Cryptography.MD5CryptoServiceProvider
                        md5.ComputeHash(f)
                        hash = md5.Hash
                        For Each hashByte As Byte In hash
                            sb.Append(String.Format("{0:X1}", hashByte))
                        Next
                        System.IO.File.AppendAllText(TextBox8.Text, filePath & " - " & sb.ToString & Environment.NewLine)
                        f.Close()

Also, I didn't mean to waste your time. I'll check out VB's debug functions. :D

I'm not sure I see the real flaw in your code but try this anyway. It is more efficient if nothing else, and might fix something we're not seeing:

VB Code:

Dim hash As Byte()
Dim sb As New StringBuilder
Dim f As FileStream
Dim md5 As New MD5CryptoServiceProvider
 
For Each filePath As String In Directory.GetFiles(TextBox7.Text)
    f = New FileStream(filePath, FileMode.Open)
    hash = md5.ComputeHash(f)
    'etc.

Thanks :D
I changed all of that, but I still have the same problem. I cant find a way to clear the hash. Even "md5.clear()" doesn't work. Here's my current code:

VB Code:

System.IO.File.AppendAllText(TextBox8.Text, "MD5 Hashes Generate by IntegriCheck" & Environment.NewLine & "Folder: " & TextBox7.Text & Environment.NewLine)
                    Dim hash As Byte()
                    Dim sb As New System.Text.StringBuilder
                    Dim f As IO.FileStream
                    Dim md5 As New MD5CryptoServiceProvider
 
                    For Each filePath As String In IO.Directory.GetFiles(TextBox7.Text)
                        f = New IO.FileStream(filePath, IO.FileMode.Open)
                        hash = md5.ComputeHash(f)
                        For Each hashByte As Byte In hash
                            sb.Append(String.Format("{0:X1}", hashByte))
                        Next
                        System.IO.File.AppendAllText(TextBox8.Text, filePath & " - " & sb.ToString & Environment.NewLine)
                        f.Close()
                    Next

How about adding this on the first line of the for each loop (and removing the new from the original declaration)

md5 = New MD5CryptoServiceProvider

Bill

I think you should definitely not call Clear, as it Disposes the object. I'm not sure why you can't transform multiple streams with the same MD5 object though.

One more note on efficiency. I'd suggest only using AppendAllText if you intend to make a single write to a file. Otherwise, open the file once at the start of the process and close it again at the end rather than opening and closing it repeatedly.

This is a little OT, but a week ago I threw together a snippet that will find all the sub folders in a path and populate them to a string array.

http://forums.aspfree.com/t87006/s.html

A little added functionality for you instead of doing each directory individually?

I don't know, just thought I'd throw it in :)

I just realised that you're using a single StringBuilder object and Appending the new hash value each time, so each value you write to the file will (should, by your code) contain all the previous values as well. Is that what you mean, as opposed to each new hash value not being created? If that's the case, I'm guessing what you actually want to do is create a new StringBuilder for each new file. Also note that the StringBuilder class has an AppendFormat method, so using String.Format is redundant and less efficient.

Thank you very much! I moved the New statement for the stringbuilder down into the loop. It works great now!

Again, using the debugger would have allowed you to solve this fairly quickly. Set a breakpoint at the top of the code, step throguh line by line and use the Autos, Locals and Watch windows to track the values of variables and expressions. Not to belabour the point, but using the debugger in the first place would have saved you a day. Glad you got it fixed though. Don't forget to resolve your thread from the Thread Tools menu.