We interrupt this LINQ series with an emergency!
Well, there is no real emergency, but there hasn’t been a WWWTC for some time, so …
The following program is suppose to compress its own source code into a Program.cs.zip file, then reverse the compression and produce a Program.txt file.
The problem is: Program.txt always shows up as an empty file!
What’s wrong?
using System;
using System.IO;
using System.IO.Compression;
namespace YippyZippy
{
class Program
{
static void Main(string[] args)
{
Compress(@"..\..\Program.cs", @"..\..\Program.cs.zip");
Decompress(@"..\..\Program.cs.zip", @"..\..\Program.txt");
}
private static void Compress(string inFileName, string outFileName)
{
FileStream inStream = File.Open(inFileName, FileMode.Open);
FileStream outStream = File.Open(outFileName, FileMode.Create);
GZipStream zipStream = new GZipStream(outStream, CompressionMode.Compress);
try
{
byte[] buffer = new byte[inStream.Length];
inStream.Read(buffer, 0, buffer.Length);
zipStream.Write(buffer, 0, buffer.Length);
}
finally
{
outStream.Close();
inStream.Close();
}
}
private static void Decompress(string inFileName, string outFileName)
{
FileStream input = File.Open(inFileName, FileMode.Open);
GZipStream zipStream = new GZipStream(input, CompressionMode.Decompress);
FileStream output = File.Open(outFileName, FileMode.Create);
try
{
int data = zipStream.ReadByte();
while (data > 0)
{
output.WriteByte((byte)data);
data = zipStream.ReadByte();
}
}
finally
{
output.Close();
input.Close();
}
}
}
}
Hint: You can fix the program by adding a single line of code.
Comments
()sserpmoC ni maertSpiz eht esolc ot togrof uoY
()esolC.maertSpiz
Winner!
so add zipStream.Close before outStream.Close() in Compress() , the code will work
Need to add zipStream.Close() in the finally section or use "using" construct instead of try/finally which explicitly calls the Dispose method of the underlying streams.
while (data > -1)
@Chris:
Good point - the code could probably change to make it clear. The ReadByte method should return an unsigned byte, meaning any value less than 0 *should* be a failure, so I think the code is safe - just isn't in tune with the documented behavior.
Josh
Just to impress on the reader that closing a GZipStream is really important. :) Even when the zip stream only looks like an intermediary between two physical files - it needs closed to flush out the compressed bytes.
If you look for -1 as Chris suggested I think you will have no trouble with any type of file(unicode/ansi/binary/etc.).
1) inStream.Read(buffer, 0, buffer.Length);
This does not do what you think it does. It doesn't read in buffer.Length number of bytes into the buffer. What it actually does is read in a number between 0 and buffer.Length bytes into the buffer and then returns you the number of bytes it has read in. You should use a binary reader if you want to get a byte array of all of the data!
2) You don't Flush or dispose the GZip Stream (I am guessing this is the error that we were ment to be looking for)
3) while (data > 0)
This should be "while (data >= 0)" zipStream.ReadByte() can quite happily return a zero as that is a perfectally valid value for a byte to be!
btw a fun blog post but I guess that is the danger with things like this is that some arogant smart arse always points out more flaws than you though you had.
DOH
Lionel
FileStream inStream ...
FileStream outStream ...
GZipStream zipStream = new GZipStream (outStream
later you do
outStream.Close();
inStream.Close();
But since the output is stored still in the buffers of zipStream which has not been flushed you will end up with an empty file if no flush of zipStream did occurs. The internal buffer size is as far as I remember 4 KB which means you will be missing the last 0-4KB of every file.
To fix this do simply change it to
zipStream.Close()
outStream.Close();
inStream.Close();
The exception handling is still a bit whacky since you do not aquire the streams inside the finally blocks and close them (if the open was successful) in the finally block if the reference was not null.
Yours,
Alois Kraus
I just want to add that zipStream.Dispose() will also work.
@Lionel: I don't need a binary reader to get an array of bytes, but one does have to be careful assuming the reader can grab all bytes at once.
Lionel
try
{
int data = zipStream.ReadByte();
while (true)
{
if (data > -1)
{
output.WriteByte((byte)data);
}
data = zipStream.ReadByte();
if (data == -1)
{
break;
}
}
}
finally
{
output.Close();
input.Close();
}