arun11299 / cpp-jwt

JSON Web Token library for C++
MIT License
395 stars 111 forks source link

Tons of warnings about comparison between signed and unsigned integers #15

Closed TheQuantumPhysicist closed 6 years ago

TheQuantumPhysicist commented 6 years ago

There are over 10 warnings on comparison between signed and unsigned integers. My compiler sees them by default; if you can't, use the flag -Wsign-compare. Some times it's because of auto, and others just direct comparison of int with sizes.

It's quite a good idea to explicitly define these types and avoid these warnings. I always try to make sure that my warnings list is empty, so that I can see serious things when they happen.

TheQuantumPhysicist commented 6 years ago

OK. It turned out not to be in lots of places, but two places that are repeated in many warnings. Here's the patch to fix this (I ensured the tests still pass):

diff --git a/include/jwt/base64.hpp b/include/jwt/base64.hpp
index 2f2a646..fcf99d6 100644
--- a/include/jwt/base64.hpp
+++ b/include/jwt/base64.hpp
@@ -94,7 +94,7 @@ std::string base64_encode(const char* in, size_t len)

   int i = 0;
   int j = 0;
-  for (; i < len - 2; i += 3) {
+  for (; i < static_cast<int>(len) - 2; i += 3) {
     const auto first  = in[i];
     const auto second = in[i+1];
     const auto third  = in[i+2];
diff --git a/include/jwt/impl/algorithm.ipp b/include/jwt/impl/algorithm.ipp
index c9fe51a..55d6591 100644
--- a/include/jwt/impl/algorithm.ipp
+++ b/include/jwt/impl/algorithm.ipp
@@ -306,9 +306,9 @@ std::string PEMSign<Hasher>::public_key_ser(

   ECDSA_SIG_get0(ec_sig.get(), &ec_sig_r, &ec_sig_s);

-  auto r_len = BN_num_bytes(ec_sig_r);
-  auto s_len = BN_num_bytes(ec_sig_s);
-  auto bn_len = (degree + 7) / 8;
+  int r_len = BN_num_bytes(ec_sig_r);
+  int s_len = BN_num_bytes(ec_sig_s);
+  int bn_len = static_cast<int>((degree + 7) / 8);

   if ((r_len > bn_len) || (s_len > bn_len)) {
     ec = AlgorithmErrc::SigningErr;
arun11299 commented 6 years ago

Thanks again!!

Would you like to create a pull request for this patch so that I can commit your changes directly ?

TheQuantumPhysicist commented 6 years ago

Done!

Cheers!

TheQuantumPhysicist commented 6 years ago

I have found more similar warnings. I'll make another pull request.