aremily commented on issue #94: Crypto 137 windows compile URL: https://github.com/apache/commons-crypto/pull/94#issuecomment-525085354 I checked in my edits. I think I caught everything. I added the console output from the test runs in the different environments to assist with your review. Thanks again, Alex On Mon, Jul 22, 2019 at 8:46 PM Marcelo Vanzin <notificati...@github.com> wrote: > *@vanzin* commented on this pull request. > > Sorry, I've seen you asking for reviews, I just don't have much time to do > a proper review of this code... much less actually test it. I did a quick > pass and didn't see anything that raised any flags (aside from being a > little hard to review the locking changes, but it looks like it's mostly > just moving code around). > ------------------------------ > > In src/main/native/org/apache/commons/crypto/OpenSslInfoNative.c > <https://github.com/apache/commons-crypto/pull/94#discussion_r306087240>: > > > #endif > > +#ifdef UNIX > +static void get_methods(JNIEnv *env, void *openssl) > +#endif > +#ifdef WINDOWS > +static void get_methods(JNIEnv *env, HMODULE openssl) > +#endif > +{ > + LOAD_OPENSSL_VERSION_FUNCTION(dlsym_OpenSSL_version_num, env, openssl); > +#ifdef UNIX > + if (dlsym_OpenSSL_version_num() > VERSION_1_1_X) { > + LOAD_DYNAMIC_SYMBOL(dlsym_OpenSSL_version, env, openssl, "OpenSSL_version"); > > 2 vs. 4 space indent, here and in other places in this file. > ------------------------------ > > In src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c > <https://github.com/apache/commons-crypto/pull/94#discussion_r306087369>: > > > @@ -183,12 +188,18 @@ JNIEXPORT void JNICALL Java_org_apache_commons_crypto_cipher_OpenSslNative_initI > env, openssl, "EVP_CIPHER_CTX_new"); > LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_free, dlsym_EVP_CIPHER_CTX_free, \ > env, openssl, "EVP_CIPHER_CTX_free"); > - LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_reset, \ > - dlsym_EVP_CIPHER_CTX_reset, env, > - openssl, "EVP_CIPHER_CTX_reset"); > - LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_set_padding, \ > - dlsym_EVP_CIPHER_CTX_set_padding, env, \ > - openssl, "EVP_CIPHER_CTX_set_padding"); > + LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_set_padding, dlsym_EVP_CIPHER_CTX_set_padding, \ > + env, openssl, "EVP_CIPHER_CTX_set_padding"); > + LOAD_DYNAMIC_SYMBOL(__dlsym_EVP_CIPHER_CTX_ctrl, dlsym_EVP_CIPHER_CTX_ctrl, \ > + env, openssl, "EVP_CIPHER_CTX_ctrl"); > > alignment is inconsistent with previous calls (also happens below in > several calls) > ------------------------------ > > In src/main/native/org/apache/commons/crypto/org_apache_commons_crypto.h > <https://github.com/apache/commons-crypto/pull/94#discussion_r306087489>: > > > @@ -134,7 +134,7 @@ void *do_version_dlsym(JNIEnv *env, void *handle) { > > /* Microsoft C Compiler does not support the C99 inline keyword */ > #ifndef __cplusplus > -#define inline __inline; > +//#define inline __inline; > > What is the problem here? If this is broken then perhaps the whole thing > should be deleted, instead of this line commented out. > ------------------------------ > > In src/main/native/org/apache/commons/crypto/org_apache_commons_crypto.h > <https://github.com/apache/commons-crypto/pull/94#discussion_r306087537>: > > > @@ -187,6 +187,24 @@ static FARPROC WINAPI do_dlsym(JNIEnv *env, HMODULE handle, LPCSTR symbol) { > } > return func_ptr; > } > + > +static FARPROC WINAPI do_version_dlsym(JNIEnv *env, HMODULE handle) { > + FARPROC func_ptr = NULL; > + if (!env || !handle) { > + THROW(env, "java/lang/InternalError", NULL); > + return NULL; > > indentation > ------------------------------ > > In src/main/native/org/apache/commons/crypto/org_apache_commons_crypto.h > <https://github.com/apache/commons-crypto/pull/94#discussion_r306087562>: > > > @@ -187,6 +187,24 @@ static FARPROC WINAPI do_dlsym(JNIEnv *env, HMODULE handle, LPCSTR symbol) { > } > return func_ptr; > } > + > +static FARPROC WINAPI do_version_dlsym(JNIEnv *env, HMODULE handle) { > + FARPROC func_ptr = NULL; > + if (!env || !handle) { > + THROW(env, "java/lang/InternalError", NULL); > + return NULL; > + } > + func_ptr = GetProcAddress(handle, "OpenSSL_version_num"); > + if (func_ptr == NULL) { > + func_ptr = GetProcAddress(handle, "SSLeay"); > + } > + return func_ptr; > +} > +/* A macro to dlsym the appropriate OpenSSL version number function. */ > > Add empty line before. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/apache/commons-crypto/pull/94?email_source=notifications&email_token=ABUDY7RW6TSNNND3PNMUJ73QAZIH5A5CNFSM4HFCEMAKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB7GXEKA#pullrequestreview-265122344>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABUDY7T6OU3BER7QVYIQYCLQAZIH5ANCNFSM4HFCEMAA> > . >
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
