Added SQLite header parsing functionality and associated tests by ptrks · Pull Request #249 · SRombauts/SQLiteCpp (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits6 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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.
@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!
Coverage remained the same at 100.0% when pulling fe14559 on patrick--:add_header_info into 54c7a18 on SRombauts:master.
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
@SRombauts you mentioned leaving some comments on the code? I might be blind, but I'm not seeing those anywhere.
| return h; |
|---|
| } |
| const SQLite::Exception exception("Could not open database, the aFilename parameter was empty."); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| if (fileBuffer.is_open()) |
|---|
| { |
| fileBuffer.seekg(0, std::ios::beg); |
| fileBuffer.read(reinterpret_cast<char*>(&buf[0]), 100); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]);
| } |
|---|
| // 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) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly here, headerStr could simply be a char[] variable directly on the header structure
| // 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"); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or encrypted database
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!