Closed Bug 1009905 Opened 5 years ago Closed 5 years ago

Wsign-compare build warning for pkixder_pki_types_tests.cpp

Categories

(Core :: Security: PSM, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1007708

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Noticed this chunk of buildspew, all for one warning about a pkix test file, using clang 3.5 with current mozilla-central:
{
 0:03.71 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.13-11-22.16-03/obj/dist/include/gtest/gtest.h: comparison of integers of different signs: 'const int' and 'const unsigned int'
 0:03.71 ../../../../dist/include/gtest/gtest.h:1316:16: warning: comparison of integers of different signs: 'const int' and 'const unsigned int' [-Wsign-compare]
 0:03.71   if (expected == actual) {
 0:03.71       ~~~~~~~~ ^  ~~~~~~
 0:03.71 ../../../../dist/include/gtest/gtest.h:1392:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned int>' requested here
 0:03.71     return CmpHelperEQ(expected_expression, actual_expression, expected,
 0:03.71            ^
 0:03.71 /scratch/work/builds/mozilla-central/mozilla-central.13-11-22.16-03/mozilla/security/pkix/test/gtest/pkixder_pki_types_tests.cpp:59:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<true>::Compare<int, unsigned int>' requested here
 0:03.71   ASSERT_EQ(0, algorithmID.parameters.len);
 0:03.71   ^
 0:03.71 ../../../../dist/include/gtest/gtest.h:1880:32: note: expanded from macro 'ASSERT_EQ'
 0:03.71 # define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2)
 0:03.71                                ^
 0:03.71 ../../../../dist/include/gtest/gtest.h:1863:67: note: expanded from macro 'GTEST_ASSERT_EQ'
 0:03.71                       EqHelper<GTEST_IS_NULL_LITERAL_(expected)>::Compare, \
 0:03.71                                                                   ^
 0:03.71 ../../../../dist/include/gtest/gtest_pred_impl.h:166:23: note: expanded from macro 'ASSERT_PRED_FORMAT2'
 0:03.71   GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_)
 0:03.71                       ^
 0:03.71 ../../../../dist/include/gtest/gtest_pred_impl.h:147:17: note: expanded from macro 'GTEST_PRED_FORMAT2_'
 0:03.71   GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2),\
 0:03.71                 ^
 0:03.71 ../../../../dist/include/gtest/gtest_pred_impl.h:77:52: note: expanded from macro 'GTEST_ASSERT_'
 0:03.71   if (const ::testing::AssertionResult gtest_ar = (expression)) \
 0:03.71                                                    ^
 0:03.80 1 warning generated.
}

This is pointing to this line:
> 59   ASSERT_EQ(0, algorithmID.parameters.len);
and it's complaining because the integer literal 0 is treated as signed, whereas algorithmID.parameters.len is unsigned.

If I simply replace 0 with 0u, the compiler is happy.
Attached patch fix (obsolete) — Splinter Review
Attachment #8422059 - Flags: review?(cviecco)
Attached patch fix v2Splinter Review
This version fixes the same warning in another .cpp test file alongside the first one.

With this version, the directory is warning-free for me. (I'm going to file a separate bug on labeling it as such.)
Attachment #8422059 - Attachment is obsolete: true
Attachment #8422059 - Flags: review?(cviecco)
Attachment #8422062 - Flags: review?(cviecco)
Attachment #8422062 - Flags: review?(cviecco)
d'oh, cpeterson's already fixing this over in bug 1007708. Duping.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1007708
You need to log in before you can comment on or make changes to this bug.