I'm working on obtaining a clean compile with -Wsign-conversion
for existing library code. The library is 25 years old or so. Eventually, this higher bar will be a security gate, and all check-ins will need to meet it or be rejected.
The existing library code does a fair amount of the following (a gross simplification that captures the essence):
class SHA1 : <inherited interfaces (contracts) and classes (behaviors)>
{
...
enum {DIGEST_SIZE = 20U};
unsigned int MaxDigestSize() { return DIGEST_SIZE; }
...
int m_digestSize;
}
Then, the library does things like (the interaction is more complex, but this captures the essence):
if(m_digestSize == -1)
m_digestSize = MaxDiegstSize();
External callers may use it like:
SHA1 sha;
...
// Calculate truncated digest
sha.Final(buffer, 8 /*signed or unsigned*/);
// Calculate using full digest
sha.Final(buffer, -1 /*signed*/);
// Calculate using full digest
sha.Final(buffer, sha.MaxDigestSize() /*unsigned*/);
The reason -1
is popular is because the following "just works" (notice the switch from SHA-1
to SHA-512
):
SHA512 sha;
// Calculate using full digest
sha.Final(buffer, -1 /*signed*/);
For the library, we want to convert to unsigned
types to squash the warning. Then, setup an enum {DEFAULT_DIGEST_VALUE=...};
to handle the case of "default digest" that was compatible with the existing "-1 pattern". Finally, remove the compares to -1
and use a compare with DEFAULT_DIGEST_VALUE
.
But enum {DEFAULT_DIGEST_VALUE=std::numeric_limits<unsigned int>::max()};
turned out to be a dead end due to limitations in numeric_limits
.
Someone suggested to use UINT_MAX
, but I'm not sure if I should use it (OS X lacks <cstdint>
, so it introduces a C header dependency). Other potential candidates are __INT_MAX__ * 2U + 1
(from <limits>
on OS X), and ~(0)
(popular in lots of library code, but maybe not correct). In fact, I'm not sure this is the proper path in general.
What is the proper or best way to:
- switch to unsigned for consistency and eventual security gate
- handle the "-1 pattern" for existing, non-library callers
- provide a portable, C++ solution
- address potential issues with bit patterns
My apologies for asking a simple question. I am a security architect by trade. I need the help of the software architects because I don't have the software engineering experience.
From the comments, here's some more information regarding constraints:
Existing Code - The library already exists, and its called Crypto++. It was authored in the 1990s, and it enjoys continued development. It is cross platform, and runs on the BSDs, Linux, OS X, Solaris, Windows. The solution needs to support older standards like C++98 and C++03. We can drop support for C++98 if the community agrees.
ABI Changes - we can change the API if the community agrees. I'm fairly certain that will happen when we make the policy change of a "clean compile as a security gate." We don't need to have 100% compatibility, but we do need to handle an existing code base of users using -1
in the field. We can even give users warning to modify their code with the preprocessor macro CRYPTOPP_MAINTAIN_BACKWARDS_COMPATIBILITY
. But in the end, things have to "just work" for them.
-Wall
. I'm slowly working my way to-Wextra
and-Wconversion
. Its sooo painful when a project does not start with them :) Both static and dynamic analysis are steps we have planned for engineering process improvements.