Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial commit of clang tidy #191

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

SalusaSecondus
Copy link
Contributor

Issue #, if available: #133

Description of changes:
Initial commit of clang tidy. I'm adding this way since I'm not sure if clang-tidy is an available dependency on all of our build systems right now.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@SalusaSecondus SalusaSecondus force-pushed the clang_tidy branch 2 times, most recently from b1c3e3a to c4fd448 Compare April 13, 2022 17:58
Copy link
Contributor

@WillChilds-Klein WillChilds-Klein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a sample diff of the repo after these linter rules have been applied?

@@ -156,10 +156,13 @@ task executeCmake(type: Exec) {
if (System.properties['TEST_JAVA_MAJOR_VERSION'] != null) {
args '-DTEST_JAVA_MAJOR_VERSION=' + System.properties['TEST_JAVA_MAJOR_VERSION']
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any way we can model the new clang-tidy dep in the dependencies section on line 49?

if not, perhaps we can add an install line to the Dockerfile.dev?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Those dependencies all come from Maven, so no. There may be a way to do it, but I'd prefer to punt for now while we figure it out.
  2. If you point me to that, then yes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good. the dockerfile currently exists here on the aws-lc branch.

CMakeLists.txt Show resolved Hide resolved
.clang-format Show resolved Hide resolved
@SalusaSecondus
Copy link
Contributor Author

diff --git a/csrc/ec_gen.cpp b/csrc/ec_gen.cpp
index 13b55ed4c..ea8b70eb5 100644
--- a/csrc/ec_gen.cpp
+++ b/csrc/ec_gen.cpp
@@ -1,15 +1,15 @@
 // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+#include "bn.h"
+#include "buffer.h"
+#include "env.h"
+#include "generated-headers.h"
+#include "util.h"
 #include <openssl/bn.h>
 #include <openssl/ec.h>
 #include <openssl/evp.h>
 #include <memory>
-#include "generated-headers.h"
-#include "util.h"
-#include "env.h"
-#include "bn.h"
-#include "buffer.h"
 
 using namespace AmazonCorrettoCryptoProvider;
 
@@ -18,9 +18,9 @@ using namespace AmazonCorrettoCryptoProvider;
  * Method:    buildEcParams
  * Signature: (I)J
  */
-JNIEXPORT jlong JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_buildEcParams
-(JNIEnv *pEnv, jclass, jint nid) {
-    EVP_PKEY_CTX *paramCtx = nullptr;
+JNIEXPORT jlong JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_buildEcParams(JNIEnv* pEnv, jclass, jint nid)
+{
+    EVP_PKEY_CTX* paramCtx = nullptr;
     jlong retval;
 
     try {
@@ -38,13 +38,13 @@ JNIEXPORT jlong JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_buildEcPa
             throw java_ex::from_openssl(EX_RUNTIME_CRYPTO, "Unable to set curve");
         }
 
-        EVP_PKEY *param = NULL;
+        EVP_PKEY* param = NULL;
         if (!EVP_PKEY_paramgen(paramCtx, &param)) {
             throw java_ex::from_openssl(EX_RUNTIME_CRYPTO, "Unable to generate parameters");
         }
 
-        retval = (jlong) param;
-    } catch (java_ex &ex) {
+        retval = (jlong)param;
+    } catch (java_ex& ex) {
         ex.throw_to_java(pEnv);
         retval = 0;
     }
@@ -58,9 +58,9 @@ JNIEXPORT jlong JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_buildEcPa
  * Method:    freeEcParams
  * Signature: (J)V
  */
-JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_freeEcParams
-(JNIEnv *env, jclass, jlong param) {
-    EVP_PKEY_free((EVP_PKEY*) param);
+JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_freeEcParams(JNIEnv* env, jclass, jlong param)
+{
+    EVP_PKEY_free((EVP_PKEY*)param);
 }
 
 // Converts from OpenSSL EC_KEY and EC_GROUP objects to jbyteArrays.
@@ -69,36 +69,32 @@ JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_freeEcPara
 // This does not currently return a success value because it is currently only
 // used as the last call in methods where the control flow does not vary
 // based on the success of this method.
-void opensslEcKey2Jarrs
-(raii_env &env,
- const EC_KEY* ecKey,
- const EC_GROUP* group,
- jbyteArray xArr,
- jbyteArray yArr,
- jbyteArray sArr) {
+void opensslEcKey2Jarrs(
+    raii_env& env, const EC_KEY* ecKey, const EC_GROUP* group, jbyteArray xArr, jbyteArray yArr, jbyteArray sArr)
+{
 
     BigNumObj xBN = bn_zero();
     BigNumObj yBN = bn_zero();
 
     // const pointers returned by get0 methods do not need to be freed
-    const BIGNUM *sBN = NULL;
-    const EC_POINT *pubKey = NULL;
-    const EC_METHOD *method = NULL;
+    const BIGNUM* sBN = NULL;
+    const EC_POINT* pubKey = NULL;
+    const EC_METHOD* method = NULL;
 
     // Convert the key into numbers
     CHECK_OPENSSL(pubKey = EC_KEY_get0_public_key(ecKey));
 
     // Figure out if this is a prime or a binary field, because they use different methods
-    method = EC_GROUP_method_of((EC_GROUP*) group);
+    method = EC_GROUP_method_of((EC_GROUP*)group);
     switch (EC_METHOD_get_field_type(method)) {
-        case NID_X9_62_prime_field:
-            CHECK_OPENSSL(EC_POINT_get_affine_coordinates_GFp((EC_GROUP*) group, pubKey, xBN, yBN, NULL) == 1);
-            break;
-        case NID_X9_62_characteristic_two_field:
-            CHECK_OPENSSL(EC_POINT_get_affine_coordinates_GF2m((EC_GROUP*) group, pubKey, xBN, yBN, NULL) == 1);
-            break;
-        default:
-            throw_java_ex(EX_RUNTIME_CRYPTO, "Unknown curve type");
+    case NID_X9_62_prime_field:
+        CHECK_OPENSSL(EC_POINT_get_affine_coordinates_GFp((EC_GROUP*)group, pubKey, xBN, yBN, NULL) == 1);
+        break;
+    case NID_X9_62_characteristic_two_field:
+        CHECK_OPENSSL(EC_POINT_get_affine_coordinates_GF2m((EC_GROUP*)group, pubKey, xBN, yBN, NULL) == 1);
+        break;
+    default:
+        throw_java_ex(EX_RUNTIME_CRYPTO, "Unknown curve type");
     }
 
     bn2jarr(env, xArr, xBN);
@@ -114,33 +110,32 @@ void opensslEcKey2Jarrs
  * Method:    generateEcKey
  * Signature: (I[B[B[B)V
  */
-JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEcKey(
-  JNIEnv *pEnv,
-  jclass,
-  jlong param,
-  jlong group,
-  jboolean checkConsistency,
-  jbyteArray xArr,
-  jbyteArray yArr,
-  jbyteArray sArr)
+JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEcKey(JNIEnv* pEnv,
+    jclass,
+    jlong param,
+    jlong group,
+    jboolean checkConsistency,
+    jbyteArray xArr,
+    jbyteArray yArr,
+    jbyteArray sArr)
 {
-    EVP_PKEY_CTX *keyCtx = NULL;
-    EVP_PKEY *pkey = NULL;
-    EC_KEY *ecKey = NULL;
+    EVP_PKEY_CTX* keyCtx = NULL;
+    EVP_PKEY* pkey = NULL;
+    EC_KEY* ecKey = NULL;
 
     try {
         raii_env env(pEnv);
 
         // Actually set up the key
-        CHECK_OPENSSL(keyCtx = EVP_PKEY_CTX_new((EVP_PKEY*) param, NULL));
+        CHECK_OPENSSL(keyCtx = EVP_PKEY_CTX_new((EVP_PKEY*)param, NULL));
         CHECK_OPENSSL(EVP_PKEY_keygen_init(keyCtx) > 0);
         CHECK_OPENSSL(EVP_PKEY_keygen(keyCtx, &pkey));
         CHECK_OPENSSL(ecKey = EVP_PKEY_get1_EC_KEY(pkey));
         if (checkConsistency) {
             CHECK_OPENSSL(EC_KEY_check_key(ecKey) == 1);
         }
-        opensslEcKey2Jarrs(env, ecKey, (EC_GROUP*) group, xArr, yArr, sArr);
-    } catch (java_ex &ex) {
+        opensslEcKey2Jarrs(env, ecKey, (EC_GROUP*)group, xArr, yArr, sArr);
+    } catch (java_ex& ex) {
         ex.throw_to_java(pEnv);
     }
 
@@ -150,19 +145,18 @@ JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEc
     EVP_PKEY_CTX_free(keyCtx);
 }
 
-JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEcKeyFromSpec(
-  JNIEnv *pEnv,
-  jclass,
-  jbyteArray paramsDer,
-  jboolean checkConsistency,
-  jbyteArray xArr,
-  jbyteArray yArr,
-  jbyteArray sArr)
+JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEcKeyFromSpec(JNIEnv* pEnv,
+    jclass,
+    jbyteArray paramsDer,
+    jboolean checkConsistency,
+    jbyteArray xArr,
+    jbyteArray yArr,
+    jbyteArray sArr)
 {
     // TODO, figure out how to do this with EVP
-    std::vector<uint8_t, SecureAlloc<uint8_t> > derBuf;
+    std::vector<uint8_t, SecureAlloc<uint8_t>> derBuf;
     EC_KEY* ecParams = NULL;
-    EC_KEY *key = NULL;
+    EC_KEY* key = NULL;
     const EC_GROUP* group;
 
     try {
@@ -175,8 +169,8 @@ JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEc
         // So, I don't feel comfortable releasing of freeing it before we're
         // completely done
         derBuf = java_buffer::from_array(env, paramsDer).to_vector(env);
-        const unsigned char* tmp = (const unsigned char*) &derBuf[0]; // necessary due to modification
-        if(!likely(ecParams = d2i_ECParameters(NULL, &tmp, derBuf.size()))) {
+        const unsigned char* tmp = (const unsigned char*)&derBuf[0]; // necessary due to modification
+        if (!likely(ecParams = d2i_ECParameters(NULL, &tmp, derBuf.size()))) {
             throw_openssl("Unable to parse parameters");
         }
 
@@ -184,7 +178,7 @@ JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEc
         CHECK_OPENSSL(group = EC_KEY_get0_group(ecParams));
 
         // Build the structure which will hold our result
-        if(!likely(key = EC_KEY_new())) {
+        if (!likely(key = EC_KEY_new())) {
             throw java_ex(EX_OOM, "Out of memory");
         }
 
@@ -199,7 +193,7 @@ JNIEXPORT void JNICALL Java_com_amazon_corretto_crypto_provider_EcGen_generateEc
         }
 
         opensslEcKey2Jarrs(env, key, group, xArr, yArr, sArr);
-    } catch (java_ex &ex) {
+    } catch (java_ex& ex) {
         ex.throw_to_java(pEnv);
     }

@alexw91
Copy link
Contributor

alexw91 commented Apr 13, 2022

I really like the diff you posted! I don't see any places where I disagree with the formatting changes.

@SalusaSecondus SalusaSecondus merged commit e15da39 into corretto:develop Apr 14, 2022
@SalusaSecondus SalusaSecondus deleted the clang_tidy branch April 14, 2022 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants