SRombauts / SQLiteCpp

SQLiteC++ (SQLiteCpp) is a smart and easy to use C++ SQLite3 wrapper.
http://srombauts.github.io/SQLiteCpp
MIT License
2.25k stars 511 forks source link

Binding BLOB requires narrowing conversion #430

Open fzakaria opened 1 year ago

fzakaria commented 1 year ago
const int lengthInBytes =
      static_cast<int>(sizeof(uint8_t) * binary->raw().size());
insert.bind(1, binary->raw().data(), lengthInBytes);

Would be nice if the API took size_t or something so that I don't have to narrow it. On my machine size_type (size_t) is unsigned long.

SRombauts commented 1 year ago

Interestingly, I am pretty sure that this was handled in the past, where I made it support short/int/long/long long etc old types from C, but it was refactored to now rely on more modern types ala uint_t.

It would make sense to perfectly support this use case and the STL in general, so I'll take a look at the current state of affairs.

What is the system you are working on?

SRombauts commented 1 year ago

For the record, I wasn't able to get a warning or a compilation error with the unit tests there are using size_t to bind a blob (tested with VS on Windows and clang on WSL)

fzakaria commented 1 year ago

Looks like it's from clang-tidy, specifically bugprone-narrowing-conversions

rc/cli/elf2sql.cc:41:40: warning: narrowing conversion from 'std::vector<unsigned char>::size_type' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]
  insert.bind(1, binary->raw().data(), binary->raw().size());
                                       ^

Sounds like a real issue and that size_t should be the type that is accepted?

SRombauts commented 1 year ago

It's not a "real-life issue" since SQLiteCpp can only bind a blob with a size in int. I created a PR nonetheless, but I'll take the time to think about it twice; it might be better in fact to NOT try to hide that and let the developer be aware of the limitations of the underlying API

tamaskenez commented 8 months ago

@SRombauts There's no such limitation, sqlite3_bind_blob64 takes sqlite3_uint64 as size.