Re: [Maria-developers] ad8b439: MDEV-9618 solaris sparc build fails on 10.1.
Hi, Alexey! On May 02, Alexey Botchkov wrote:
revision-id: ad8b4399f2fcd1d5e9dcdb8b2a8832cb3a6745ae (mariadb-10.1.13-23-gad8b439) parent(s): ad4239cc3dc7ad5f6f264e1fb3cf6d24084bda90 committer: Alexey Botchkov timestamp: 2016-05-02 12:03:39 +0400 message:
MDEV-9618 solaris sparc build fails on 10.1.
Compiler on Solaris is sensitive to C/C++ call models differences, so it fails if we try to mix these two in '?' operator. Fixed by trying to keep the call models uniformity with the proper amount of 'extern "C"' hints.
--- include/my_crypt.h | 6 ++++++ include/mysql/plugin_audit.h | 7 +++++++ include/mysql/plugin_encryption.h | 9 +++++++++ .../example_key_management_plugin.cc | 2 +- plugin/file_key_management/file_key_management_plugin.cc | 2 +- sql/encryption.cc | 14 ++++++++++---- unittest/sql/mf_iocache-t.cc | 2 +- 7 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/include/my_crypt.h b/include/my_crypt.h index e1e94c9..db280ca 100644 --- a/include/my_crypt.h +++ b/include/my_crypt.h @@ -82,6 +82,12 @@ static inline uint my_aes_ctx_size(enum my_aes_mode mode __attribute__((unused)) return MY_AES_CTX_SIZE; }
+static inline uint my_aes_ctx_size_for_handler(unsigned int a, + unsigned int b __attribute__((unused))) +{ + return my_aes_ctx_size((enum my_aes_mode) a); +} +
This won't be inlined, because you store a pointer to this function in a structure. Better use the cast, as before.
int my_random_bytes(uchar* buf, int num);
#ifdef __cplusplus diff --git a/include/mysql/plugin_audit.h b/include/mysql/plugin_audit.h index 31589f0..e96f743 100644 --- a/include/mysql/plugin_audit.h +++ b/include/mysql/plugin_audit.h @@ -23,6 +23,10 @@
#include "plugin.h"
+#ifdef __cplusplus +extern "C" { +#endif
I agree, in general. But why does your short patch have nothing about audit api?
#define MYSQL_AUDIT_CLASS_MASK_SIZE 1
#define MYSQL_AUDIT_INTERFACE_VERSION 0x0302 @@ -174,5 +178,8 @@ struct st_mysql_audit unsigned long class_mask[MYSQL_AUDIT_CLASS_MASK_SIZE]; };
+#ifdef __cplusplus +} +#endif
#endif diff --git a/include/mysql/plugin_encryption.h b/include/mysql/plugin_encryption.h index 3f35c2b..d748c4f 100644 --- a/include/mysql/plugin_encryption.h +++ b/include/mysql/plugin_encryption.h @@ -27,6 +27,10 @@
#include <mysql/plugin.h>
+#ifdef __cplusplus +extern "C" { +#endif
...etc I agree that these extern "C" are the correct fix. On the other hand, I don't see a point in moving casts from the assignment to a new variable definition. So, I'd suggest to keep all extern "C" hunks in the patch, but remove cast avoidance changes. Then ok to push! Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hello, SerG.
diff --git a/include/mysql/plugin_audit.h b/include/mysql/plugin_audit.h index 31589f0..e96f743 100644 +#ifdef __cplusplus +extern "C" { I agree, in general. But why does your short patch have nothing about audit api?
Because it doesn't lead to any build failures. So there's no actual need for this change. Added just to be consistent.
On the other hand, I don't see a point in moving casts from the assignment to a new variable definition. So, I'd suggest to keep all extern "C" hunks in the patch, but remove cast avoidance changes.
I don't know how can i do that nicely. What i tried by now - change (uint (*)(unsigned int, unsigned int))my_aes_ctx_size; with (extern "C" uint (*)(unsigned int, unsigned int))my_aes_ctx_size; that didn't compile. Then i tried to put the 'extern "C" {" around the initialize_encryption_plugin(). It worked but then i needed to add the 'extern "C"' to the initialize_encryption_plugin() declaration. What would you suggest? HF. 02.05.2016 19:32, Sergei Golubchik wrote:
Hi, Alexey!
On May 02, Alexey Botchkov wrote:
revision-id: ad8b4399f2fcd1d5e9dcdb8b2a8832cb3a6745ae (mariadb-10.1.13-23-gad8b439) parent(s): ad4239cc3dc7ad5f6f264e1fb3cf6d24084bda90 committer: Alexey Botchkov timestamp: 2016-05-02 12:03:39 +0400 message:
MDEV-9618 solaris sparc build fails on 10.1.
Compiler on Solaris is sensitive to C/C++ call models differences, so it fails if we try to mix these two in '?' operator. Fixed by trying to keep the call models uniformity with the proper amount of 'extern "C"' hints.
--- include/my_crypt.h | 6 ++++++ include/mysql/plugin_audit.h | 7 +++++++ include/mysql/plugin_encryption.h | 9 +++++++++ .../example_key_management_plugin.cc | 2 +- plugin/file_key_management/file_key_management_plugin.cc | 2 +- sql/encryption.cc | 14 ++++++++++---- unittest/sql/mf_iocache-t.cc | 2 +- 7 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/include/my_crypt.h b/include/my_crypt.h index e1e94c9..db280ca 100644 --- a/include/my_crypt.h +++ b/include/my_crypt.h @@ -82,6 +82,12 @@ static inline uint my_aes_ctx_size(enum my_aes_mode mode __attribute__((unused)) return MY_AES_CTX_SIZE; }
+static inline uint my_aes_ctx_size_for_handler(unsigned int a, + unsigned int b __attribute__((unused))) +{ + return my_aes_ctx_size((enum my_aes_mode) a); +} + This won't be inlined, because you store a pointer to this function in a structure. Better use the cast, as before.
int my_random_bytes(uchar* buf, int num);
#ifdef __cplusplus diff --git a/include/mysql/plugin_audit.h b/include/mysql/plugin_audit.h index 31589f0..e96f743 100644 --- a/include/mysql/plugin_audit.h +++ b/include/mysql/plugin_audit.h @@ -23,6 +23,10 @@
#include "plugin.h"
+#ifdef __cplusplus +extern "C" { +#endif I agree, in general. But why does your short patch have nothing about audit api?
#define MYSQL_AUDIT_CLASS_MASK_SIZE 1
#define MYSQL_AUDIT_INTERFACE_VERSION 0x0302 @@ -174,5 +178,8 @@ struct st_mysql_audit unsigned long class_mask[MYSQL_AUDIT_CLASS_MASK_SIZE]; };
+#ifdef __cplusplus +} +#endif
#endif diff --git a/include/mysql/plugin_encryption.h b/include/mysql/plugin_encryption.h index 3f35c2b..d748c4f 100644 --- a/include/mysql/plugin_encryption.h +++ b/include/mysql/plugin_encryption.h @@ -27,6 +27,10 @@
#include <mysql/plugin.h>
+#ifdef __cplusplus +extern "C" { +#endif ...etc
I agree that these extern "C" are the correct fix. On the other hand, I don't see a point in moving casts from the assignment to a new variable definition. So, I'd suggest to keep all extern "C" hunks in the patch, but remove cast avoidance changes.
Then ok to push!
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Hi, Alexey! On May 03, Alexey Botchkov wrote:
On the other hand, I don't see a point in moving casts from the assignment to a new variable definition. So, I'd suggest to keep all extern "C" hunks in the patch, but remove cast avoidance changes.
I don't know how can i do that nicely. What i tried by now - change (uint (*)(unsigned int, unsigned int))my_aes_ctx_size; with (extern "C" uint (*)(unsigned int, unsigned int))my_aes_ctx_size; that didn't compile.
Then i tried to put the 'extern "C" {" around the initialize_encryption_plugin(). It worked but then i needed to add the 'extern "C"' to the initialize_encryption_plugin() declaration.
What would you suggest?
Oh, so you mean that (uint (*)(unsigned int, unsigned int))my_aes_ctx_size assumes C++ linkage? Even if my_aes_ctx_size itself is extern "C" ? Grrr. Okay, then, perhaps, use your second patch with if/else? But still add extern "C" to include/mysql/*.h headers - that's a good thing to have. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Alexey Botchkov -
Sergei Golubchik