Fix three 'comparison between signed and unsigned integer expressions' warnings. #8331

pull dooglus wants to merge 1 commits into bitcoin:master from dooglus:fix-compilation-warnings changing 3 files +3 −3

dooglus commented at 10:35 PM on July 11, 2016: contributor

Building from the master branch I saw three warnings from the compiler:

leveldb/db/memtable.cc: In member function ‘void leveldb::MemTable::Add(leveldb::SequenceNumber, leveldb::ValueType, const leveldb::Slice&, const leveldb::Slice&)’:
leveldb/db/memtable.cc:104:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   assert((p + val_size) - buf == encoded_len);
                               ^
leveldb/util/bloom.cc: In member function ‘virtual void leveldb::{anonymous}::BloomFilterPolicy::CreateFilter(const leveldb::Slice*, int, std::string*) const’:
leveldb/util/bloom.cc:50:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     for (size_t i = 0; i < n; i++) {
                          ^
leveldb/util/logging.cc: In function ‘bool leveldb::ConsumeDecimalNumber(leveldb::Slice*, uint64_t*)’:
leveldb/util/logging.cc:58:40: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
           (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
                                    ^

Fix three 'comparison between signed and unsigned integer expressions' warnings. 63c83dd82d
sipa commented at 10:50 PM on July 11, 2016: member

Changes to the leveldb subtree need to be made in the leveldb repository (https://github.com/bitcoin-core/leveldb).

However, since this is code from a separate repository, wouldn't it be better to just pass -Wno-sign-compare to the leveldb build?
dcousens commented at 5:58 AM on July 12, 2016: contributor

However, since this is code from a separate repository, wouldn't it be better to just pass -Wno-sign-compare to the leveldb build?

Wouldn't it be better to just fix the warnings up stream?
jonasschnelli added the label Refactoring on Jul 12, 2016
jonasschnelli added the label Upstream on Jul 12, 2016

in src/leveldb/util/bloom.cc:None in 63c83dd82d

  46 | @@ -47,7 +47,7 @@ class BloomFilterPolicy : public FilterPolicy {
  47 |      dst->resize(init_size + bytes, 0);
  48 |      dst->push_back(static_cast<char>(k_));  // Remember # of probes in filter
  49 |      char* array = &(*dst)[init_size];
  50 | -    for (size_t i = 0; i < n; i++) {
  51 | +    for (int i = 0; i < n; i++) {

paveljanik commented at 5:59 PM on July 12, 2016:

At least this change is already present in the upstream: https://github.com/google/leveldb/blob/master/util/bloom.cc#L50

paveljanik commented at 6:00 PM on July 12, 2016: contributor

Yes, please these issues upstream.
MarcoFalke commented at 2:10 PM on July 16, 2016: member

We would need to ask google to do a release of leveldb which includes the fix mentioned by @paveljanik in the comment. I don't think it is worth keeping this pull open in the meantime.
laanwj commented at 9:09 AM on August 3, 2016: member

We try to deviate from upstream leveldb as little as possible, unless there is a strong reason. Preventing a few warnings is, IMO, not such. I suggest to file this change upstream, it will come to us in due time.
laanwj closed this on Aug 3, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels