Added SQLite header parsing functionality and associated tests #249
Conversation
|
@SRombauts tell me what you think of this implementation. In the meantime I am looking into getting the tests to pass but still have access to fixed width integer datatypes. |
|
About the compile error, you can use plain old C type (unsigned char/short/int). Alternatively you could apply your pull request to the slqitcpp-3.x branch that is c++11 |
… datatypes to cpp11 branch
|
@SRombauts I am going to go ahead and replace with plain C types for now so it can be available in master. I'll also open a PR with fixed width support for the C+11 branch. Thanks! |
|
I left a bunch of comments, but it looks good overall! |
|
Also don't bother to create a different version for c++11, I don't want that we go an rewrite all just for the sake of it |
|
And finally, I have just reached 100% unit test code coverage (for what it's worth) so you should test all cases of error https://coveralls.io/builds/27840818/source?filename=src/Database.cpp Basically you have to test also with no filename, no file, and wrong file header |
|
@SRombauts okay I will get those tests added. |
…nd a corrupt header
|
@SRombauts you mentioned leaving some comments on the code? I might be blind, but I'm not seeing those anywhere. |
src/Database.cpp
Outdated
| return h; | ||
| } | ||
|
|
||
| const SQLite::Exception exception("Could not open database, the aFilename parameter was empty."); |
There was a problem hiding this comment.
Could you perhaps put this exception at the start of the function like I did yesterday on a cleanup commit? It would avoid the return being inside the scope.
src/Database.cpp
Outdated
| if (fileBuffer.is_open()) | ||
| { | ||
| fileBuffer.seekg(0, std::ios::beg); | ||
| fileBuffer.read(reinterpret_cast<char*>(&buf[0]), 100); |
There was a problem hiding this comment.
I think it would be better to avoid these cast; perhaps have one additional "pointer of char" variable pointing to the buf.
unsigned char buf[100];
char* pBuf = reinterpret_cast<char*>(&buf[0]);
src/Database.cpp
Outdated
| } | ||
|
|
||
| // If the "magic string" can't be found then header is invalid, corrupt or unreadable | ||
| if (!strncmp(reinterpret_cast<const char*>(h.headerStr), "SQLite format 3", 15) == 0) |
There was a problem hiding this comment.
Similarly here, headerStr could simply be a char[] variable directly on the header structure
src/Database.cpp
Outdated
| // If the "magic string" can't be found then header is invalid, corrupt or unreadable | ||
| if (!strncmp(reinterpret_cast<const char*>(h.headerStr), "SQLite format 3", 15) == 0) | ||
| { | ||
| const SQLite::Exception exception("Invalid SQLite header"); |
|
Sorry, my bad, I didn't know this new review system needed a submit when finished |
…of function calls and cleared up invalid header exception message
|
That's very good, thanks a lot! |
Added header parsing functionality via a
getHeaderInfo()function. This function reads the first 100 bytes of a SQLite database file and attempts to reconstruct byte groups into the specific header fields within a Header object. Also added tests to demonstrate updating header values via PRAGMA and verifying updated / default header values.