maria-developers team mailing list archive

[Alexey Botchkov <holyfoot@xxxxxxxxxxxx>]

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@xxxxxxxxxxx
>
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp