ots icon indicating copy to clipboard operation
ots copied to clipboard

-Wsign-compare: warning: comparison of integer expressions of different signedness

Open gagath opened this issue 4 years ago • 3 comments

Hello,

With the default set of compiler flags on Debian, the following warnings appear:

../src/cff_charstring.cc: In function ‘bool {anonymous}::ExecuteCharStringOperator(ots::OpenTypeCFF&, int32_t, size_t, const ots::CFFIndex&, const ots::CFFIndex&, ots::Buffer*, ots::Buffer*, std::stack<int>*, bool*, bool*, size_t*, bool*, bool*, int32_t*, bool)’:
../src/cff_charstring.cc:376:31: warning: comparison of integer expressions of different signedness: ‘__gnu_cxx::__alloc_traits<std::allocator<int>, int>::value_type’ {aka ‘int’} and ‘std::vector<short unsigned int>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare]
  376 |     if (argument_stack->top() >= cff.region_index_count.size()) {
      |         ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/cff_charstring.cc:393:25: warning: comparison of integer expressions of different signedness: ‘int32_t’ {aka ‘int’} and ‘std::vector<short unsigned int>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare]
  393 |     if (*in_out_vsindex >= cff.region_index_count.size()) {
      |         ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/cff_charstring.cc:398:20: warning: comparison of integer expressions of different signedness: ‘const size_t’ {aka ‘const long unsigned int’} and ‘int’ [-Wsign-compare]
  398 |     if (stack_size < n * (k + 1) + 1) {
      |         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~
../src/cff.cc: In function ‘bool {anonymous}::ParsePrivateDictData(ots::Buffer&, size_t, size_t, {anonymous}::DICT_DATA_TYPE, ots::OpenTypeCFF*)’:
../src/cff.cc:548:29: warning: comparison of integer expressions of different signedness: ‘std::vector<std::pair<unsigned int, {anonymous}::DICT_OPERAND_TYPE> >::size_type’ {aka ‘long unsigned int’} and ‘int’ [-Wsign-compare]
  548 |         if (operands.size() < n * (k + 1) + 1) {
      |             ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

No other warnings are found, so it is quite a good news! As this code is very specific, I am not sure about what the solution would be between using int64_t everywhere, or doing something smarter.

Best regards.

gagath avatar Dec 04 '20 15:12 gagath

Here is an example G++ command-line in case that helps track down the issues:

c++ -Ilibots.a.p -I. -I.. -I../include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -std=c++11 -g -O2 -fdebug-prefix-map=/build/opentype-sanitizer-8.1.1+dfsg.1=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -DHAVE_CONFIG_H -MD -MQ libots.a.p/src_cff_charstring.cc.o -MF libots.a.p/src_cff_charstring.cc.o.d -o libots.a.p/src_cff_charstring.cc.o -c ../src/cff_charstring.cc

pabs3 avatar Dec 05 '20 02:12 pabs3

From the warnings it sounds like some of the code should use size_t for types instead of int.

pabs3 avatar Dec 05 '20 02:12 pabs3

A couple more instances:

ots/src/stat.cc(264,34): warning: comparison of integers of different signs: 'long' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
    if (out->Tell() - tableStart != this->designAxesOffset) {
        ~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~
ots/src/stat.cc(279,34): warning: comparison of integers of different signs: 'long' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare]
    if (out->Tell() - tableStart != this->offsetToAxisValueOffsets) {
        ~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

rvandermeulen avatar Jul 27 '22 12:07 rvandermeulen