Skip to content

Commit

Permalink
Remove BN_DEC_FMT2 and test the others
Browse files Browse the repository at this point in the history
Andres Erbsen noticed we didn't actually have tests to catch when the
format macros were wrong.

In doing so, remove BN_DEC_FMT2. It was unused and only makes sense in
the context of the bignum-to-decimal conversion, where we no longer use
it anyway. None of these macros are exported in OpenSSL at all, so it
should be safe to remove it. (We possibly can remove the others too. I
see one use outside the library, and that use would probably be better
written differently anyway.)

Change-Id: I4ffc7f9f7dfb399ac060af3caff0778000010303
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60325
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed Jun 2, 2023
1 parent 28c2409 commit e106b53
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
22 changes: 22 additions & 0 deletions crypto/fipsmodule/bn/bn_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
#include <string.h>

#include <algorithm>
#include <limits>
#include <utility>

#include <gtest/gtest.h>
Expand Down Expand Up @@ -2796,6 +2797,27 @@ TEST_F(BNTest, MontgomeryLarge) {
ctx(), nullptr));
}

TEST_F(BNTest, FormatWord) {
char buf[32];
snprintf(buf, sizeof(buf), BN_DEC_FMT1, BN_ULONG{1234});
EXPECT_STREQ(buf, "1234");
snprintf(buf, sizeof(buf), BN_HEX_FMT1, BN_ULONG{1234});
EXPECT_STREQ(buf, "4d2");

// |BN_HEX_FMT2| is zero-padded up to the maximum value.
#if defined(OPENSSL_64_BIT)
snprintf(buf, sizeof(buf), BN_HEX_FMT2, BN_ULONG{1234});
EXPECT_STREQ(buf, "00000000000004d2");
snprintf(buf, sizeof(buf), BN_HEX_FMT2, std::numeric_limits<BN_ULONG>::max());
EXPECT_STREQ(buf, "ffffffffffffffff");
#else
snprintf(buf, sizeof(buf), BN_HEX_FMT2, BN_ULONG{1234});
EXPECT_STREQ(buf, "000004d2");
snprintf(buf, sizeof(buf), BN_HEX_FMT2, std::numeric_limits<BN_ULONG>::max());
EXPECT_STREQ(buf, "ffffffff");
#endif
}

#if defined(SUPPORTS_ABI_TEST)
// These functions are not always implemented in assembly, but they sometimes
// are, so include ABI tests for each.
Expand Down
2 changes: 0 additions & 2 deletions include/openssl/bn.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,12 @@ extern "C" {
typedef uint64_t BN_ULONG;
#define BN_BITS2 64
#define BN_DEC_FMT1 "%" PRIu64
#define BN_DEC_FMT2 "%019" PRIu64
#define BN_HEX_FMT1 "%" PRIx64
#define BN_HEX_FMT2 "%016" PRIx64
#elif defined(OPENSSL_32_BIT)
typedef uint32_t BN_ULONG;
#define BN_BITS2 32
#define BN_DEC_FMT1 "%" PRIu32
#define BN_DEC_FMT2 "%09" PRIu32
#define BN_HEX_FMT1 "%" PRIx32
#define BN_HEX_FMT2 "%08" PRIx32
#else
Expand Down

0 comments on commit e106b53

Please sign in to comment.