maria-developers team mailing list archive

[Alexey Botchkov <holyfoot@xxxxxxxxxxxx>]

Sergei,

The complete patch is here:
http://lists.askmonty.org/pipermail/commits/2013-July/004880.html

> >  #ifndef MARIADB_ONLY
> >  #undef MYSQL_SERVICE_LOGGER_INCLUDED
> >  #undef MYSQL_DYNAMIC_PLUGIN

> ^^^ why undefs?

I want the service_logger.h to be included in no-mysql-dynamic-plugin mode.
But the rest of the services should be included normally, as i belive.
So in the code i first do #define MYSQL_SERVICE_LOGGER_INCLUDED, so
the #include <services.h> didn't actually included the service_logger.h.

Then I do the #undef and #include <service_logger.h> separately.

> Could you please move HASH into a service?

Ok, will do right after this task (so it won't be forgotten).
But yes, I think it's better to do it separately.

Best regards.
HF


02.07.2013 14:34, Sergei Golubchik wrote:
> Hi, Alexey!
>
> On Jul 01, Alexey Botchkov wrote:
> > Hi, Serg.
> >
> > Here is the last version of the plugin for your review:
> > http://myoffice.izhnet.ru/~alex/server_audit/server_audit.c
> > <http://myoffice.izhnet.ru/%7Ealex/server_audit/server_audit.c>
> >
> > I think I addressed there everything we discussed.
> Could you send a complete patch instead, please?
> Looks ok, a couple of comments:
>
>    #ifndef MARIADB_ONLY
>    #undef MYSQL_SERVICE_LOGGER_INCLUDED
>    #undef MYSQL_DYNAMIC_PLUGIN
>
> ^^^ why undefs?
> I wouldn't need to ask this question, if I'd have a complete patch,
> that includes file_logger.c
>
>    ...
>      if (gethostname(servhost, sizeof(servhost)))
>        strcpy(servhost, "NA");
> ^^^
> It's usually written as "N/A" and means "not applicable". But the host
> name here is most certainly applicable. Better use "???" or "unknown" or
> even "localhost".
>
> And two more comments:
> 1. I presume the complete patch includes test cases for the new plugin.
> 2. Could you please move HASH into a service? In a separate changeset,
>     of course. Moving an existing function into a service does not change
>     the source code of plugins, so you can do it after you've pushed
>     this audit plugin, if you'd like.
>
> Regards,
> Sergei