Return to Answer

• I see that you already use the bool type, so extend that to the return type of functions that only have two possible outcomes, success (true) and failure (false). Returning integer error codes is error prone for the programmer since there isn't a single agreement or convention on which values should be returned. A boolean is much more obvious of its meaning.

• Go easy on the magic numbers. You have a lot of repeated literal constants out there that could be consolidated into a few named constants. #define and enum are the traditional C ways of doing that, but I'd instead recommend preferring static consts whenever possible to gain some type safety.

• You correctly use stderr in your decompression routines to print errors, but in main() all error reporting is done with printf, which outputs to stdout. Error output should always be sent to stderr, so that users can easily filter the program output. Replace those error prints with stderr. You might also consider adding the output of strerror() to the messages involving system errors (like fopen() and friends).

• I see that you already use the bool type, so extend that to the return type of functions that only have two possible outcomes, success (true) and failure (false). Returning integer error codes is error prone for the programmer since there isn't a single agreement or convention on which values should be returned. A boolean is much more obvious of its meaning.

• Go easy on the magic numbers. You have a lot of repeated literal constants out there that could be consolidated into a few named constants. #define and enum are the traditional C ways of doing that, but I'd instead recommend preferring static consts whenever possible to gain some type safety.

• You correctly use stderr in your decompression routines to print errors, but in main() all error reporting is done with printf, which outputs to stdout. Error output should always be sent to stderr, so that users can easily filter the program output. Replace those error prints with stderr. You might also consider adding the output of strerror() to the messages involving system errors (like fopen() and friends).

After this change is applied, be sure to properly initialize the the structure. Your code probably relies on the globals being zero initialized. The compiler does that for you for global variables, but a struct declared at function scope is not cleared by default. So you might memset() the LzwCompression instance either on main() or right at the top of decompress(). Also set the left field to true after memseting the structure, since that was the starting value of the global originally.

Well, that's all the juice I have ;). Compression is not my field, so I don't know how you can improve the algorithm. Hopefully someone else will provide some comments on that.

If I follow correctly the logic of decompress(), that 1 byte malloc you did in the above function gets overwritten without ever being freed. But actually, mallocing a single byte of memory is very inefficient, so your should try to refactor that to instead perform a single allocation of the sum of all bytes. I'll leave that as an exercise to you ;).

Well, that's all the juice I have! Compression is not my field, so I don't know how you can improve the algorithm. Hopefully someone else will provide some comments on that.

If I follow correctly the logic of decompress(), that 1 byte malloc you did in the above function gets overwritten without ever being freed. But actually, mallocing a single byte of memory is very inefficient, so you should try to refactor that to instead perform a single allocation of the sum of all bytes. I'll leave that as an exercise ;).