// At the moment
if(strcmp(firstcheck, "MPF") != 0){
// But should acutally be
if(strncmp(firstcheck, "MPF", 3) != 0){
Actually, it should be
if(memcmp(firstcheck, "MPF", 3) != 0){ to emphasize it's really a binary comparison.
Can I nitpick some more?
So, why on Earth earlier in the same function it's
char* firstcheck;
firstcheck = new char[3];and not the vastly simpler
char firstcheck[3];?
Why there are so much unadorned new[] and delete[]s in the code? Familiarize yourself with RAII and classes like std::vector or boost::scoped_array. It'll make the code simpler, easier to read and write, and more correct. Are you sure there are not memory leaks? I'm not. Given that there are so many ways for things to go wrong, I wouldn't
dare to ignore RAII.
Why all the loop counters are of type
int, not
container::size_type (in most cases the same as
std::size_t)? You not only have to cast to silence a warning about comparison between signed (int) and unsigned (size_t), you also make the code not work for all sizes larger than INT_MAX. On some common platforms, size_t is in fact larger than int, so the code may crash in a funny way.
Why all the std::strings are passed to functions by value? Pass by a reference to a constant, you'll get rid of unnecessary copying. It's very common idiom, so everyone will understand what you mean.
Why you're returning after throwing an exception? These returns are unreachable, throw already changes the control flow so much you probably are in a completely different function now, so the returns will have no chance to get executed.
Why I'm nitpicking on such issues? On this forum it's Nexus's job to do that, not mine
Anyway, it's a nice library, keep on working on it, even though it uses it's own format instead of something widely supported. Could you document the details about the package's binary structure?
