Re: [Maria-developers] Rev 2734: Maria WL#61
29 Mar
2010
29 Mar
'10
9:39 p.m.
Hi, Serg!
23 марта 2010, в 21:53, Sergei Golubchik написал(а):
> Hi, Sanja!
>
> See the review below.
> But please note that wl:43 and red 5.2 have higher priority, don't
> start
> on this one until you're ready with wl:43 and fixed the soft_sync_rwl
> problem in 5.2.
>
> On Mar 11, sanja@askmonty.org wrote:
>
>> === modified file 'include/mysql/plugin.h'
>> --- a/include/mysql/plugin.h 2009-09-07 20:50:10 +0000
>> +++ b/include/mysql/plugin.h 2010-03-11 15:02:03 +0000
>> @@ -86,6 +89,21 @@
>> #define PLUGIN_LICENSE_GPL_STRING "GPL"
>> #define PLUGIN_LICENSE_BSD_STRING "BSD"
>>
>> +/* definitions of code maturity for plugins */
>> +#define PLUGIN_MATURITY_UNKNOWN 0
>
> make it MariaDB_PLUGIN_MATURITY_...
OK
>
>> +#define PLUGIN_MATURITY_TEST 1
>> +#define PLUGIN_MATURITY_ALPHA 2
>> +#define PLUGIN_MATURITY_BETA 3
>> +#define PLUGIN_MATURITY_GAMMA 4
>> +#define PLUGIN_MATURITY_RELEASE 5
>
> "release" ? or production or stable ?
> there can be an alpha release, or beta release.
> obviously, "release" does not indicate a degree of maturity.
>
> and I don't like "test" either.
> PLUGIN_MATURITY_TEST is just not readable. I'd rather drop it and
> start
> with alpha.
I agree with changing RELEASE with STABLE, but test looks logical for
me (and for Monty also as far he invented it).
>> +
>> +#define PLUGIN_MATURITY_UNKNOWN_STR "Unknown"
>> +#define PLUGIN_MATURITY_TEST_STR "Test"
>> +#define PLUGIN_MATURITY_ALPHA_STR "Alpha"
>> +#define PLUGIN_MATURITY_BETA_STR "Beta"
>> +#define PLUGIN_MATURITY_GAMMA_STR "Gamma"
>> +#define PLUGIN_MATURITY_RELEASE_STR "Release"
>> +
>
> why did you put these macros in the plugin.h ?
> as far as I can see there's no need to make them part of the
> interface,
> they can be completely internal
OK
>> /*
>> Macros for beginning and ending plugin declarations. Between
>> mysql_declare_plugin and mysql_declare_plugin_end there should
>> @@ -94,15 +112,29 @@
>>
>>
>> #ifndef MYSQL_DYNAMIC_PLUGIN
>> +
>> #define __MYSQL_DECLARE_PLUGIN(NAME, VERSION, PSIZE,
>> DECLS) \
>> int VERSION=
>> MYSQL_PLUGIN_INTERFACE_VERSION; \
>> int PSIZE= sizeof(struct
>> st_mysql_plugin); \
>> struct st_mysql_plugin DECLS[]= {
>> +
>> +#define __MARIA_DECLARE_PLUGIN(NAME, VERSION, PSIZE,
>> DECLS) \
>
> don't use names that start from __, the C99 standard says
> "All identifiers that begin with an underscore and either an
> uppercase letter
> or another underscore are always reserved for any use."
> (ISO/IEC 9899:1999, 7.1.3 Reserved Identifiers)
>
> It's unfortunate that __MYSQL_DECLARE_PLUGIN breaks this rule,
> but it's not a reason for us to do the same
So what will be better, one or three or remove '_' at all?
>> +int VERSION=
>> MARIA_PLUGIN_INTERFACE_VERSION; \
>> +int PSIZE= sizeof(struct
>> st_maria_plugin); \
>> +struct st_maria_plugin DECLS[]= {
>> +
>> #else
>> +
>> #define __MYSQL_DECLARE_PLUGIN(NAME, VERSION, PSIZE,
>> DECLS) \
>> MYSQL_PLUGIN_EXPORT int _mysql_plugin_interface_version_=
>> MYSQL_PLUGIN_INTERFACE_VERSION; \
>> MYSQL_PLUGIN_EXPORT int _mysql_sizeof_struct_st_plugin_=
>> sizeof(struct st_mysql_plugin); \
>> MYSQL_PLUGIN_EXPORT struct st_mysql_plugin
>> _mysql_plugin_declarations_[]= {
>> +
>> +#define __MARIA_DECLARE_PLUGIN(NAME, VERSION, PSIZE,
>> DECLS) \
>> +MYSQL_PLUGIN_EXPORT int _maria_plugin_interface_version_=
>> MARIA_PLUGIN_INTERFACE_VERSION; \
>> +MYSQL_PLUGIN_EXPORT int _maria_sizeof_struct_st_plugin_=
>> sizeof(struct st_maria_plugin); \
>> +MYSQL_PLUGIN_EXPORT struct st_maria_plugin
>> _maria_plugin_declarations_[]= {
>> +
>> #endif
>>
>> #define mysql_declare_plugin(NAME) \
>> @@ -407,6 +446,31 @@
>> void * __reserved1; /* reserved for dependency
>> checking */
>> };
>>
>> +/*
>> + MariaDB extension for plugins declaration structure.
>> +
>> + It also copy current MySQL plugin fields to have more independency
>> + in plugins extension
>> +*/
>> +
>> +struct st_maria_plugin
>> +{
>> + int type; /* the plugin type (a MYSQL_XXX_PLUGIN
>> value) */
>> + void *info; /* pointer to type-specific plugin
>> descriptor */
>> + const char *name; /* plugin
>> name */
>> + const char *author; /* plugin author (for SHOW
>> PLUGINS) */
>> + const char *descr; /* general descriptive text (for SHOW
>> PLUGINS ) */
>> + int license; /* the plugin license
>> (PLUGIN_LICENSE_XXX) */
>> + int (*init)(void *); /* the function to invoke when plugin is
>> loaded */
>> + int (*deinit)(void *);/* the function to invoke when plugin is
>> unloaded */
>> + unsigned int version; /* plugin version (for SHOW
>> PLUGINS) */
>> + struct st_mysql_show_var *status_vars;
>> + struct st_mysql_sys_var **system_vars;
>> + const char *version_info; /* plugin version string */
>> + int maturity; /* HA_PLUGIN_MATURITY_XXX */
>> + void * __reserved1; /* reserved for dependency
>> checking */
>
> remove this __reserved1 field
OK
>> +};
>> +
>> /
>> *************************************************************************
>> API for Full-text parser plugin. (MYSQL_FTPARSER_PLUGIN)
>> */
>> === modified file 'sql/ha_ndbcluster.cc'
>> --- a/sql/ha_ndbcluster.cc 2009-09-07 20:50:10 +0000
>> +++ b/sql/ha_ndbcluster.cc 2010-03-11 15:02:03 +0000
>> @@ -10561,5 +10561,23 @@
>> NULL /* config options */
>> }
>> mysql_declare_plugin_end;
>> +maria_declare_plugin(ndbcluster)
>> +{
>> + MYSQL_STORAGE_ENGINE_PLUGIN,
>> + &ndbcluster_storage_engine,
>> + ndbcluster_hton_name,
>> + "MySQL AB",
>> + "Clustered, fault-tolerant tables",
>> + PLUGIN_LICENSE_GPL,
>> + ndbcluster_init, /* Plugin Init */
>> + NULL, /* Plugin Deinit */
>> + 0x0100 /* 1.0 */,
>> + ndb_status_variables_export,/* status variables */
>> + NULL, /* system variables */
>> + "1.0", /* string version */
>> + PLUGIN_MATURITY_BETA, /* maturity */
>
> beta ? says who ?
My guess reviewed by Monty.
>> + NULL /* config options */
>> +}
>> +maria_declare_plugin_end;
>>
>> #endif
>> === modified file 'sql/sql_builtin.cc.in'
>> --- a/sql/sql_builtin.cc.in 2006-12-31 01:29:11 +0000
>> +++ b/sql/sql_builtin.cc.in 2010-03-11 15:02:03 +0000
>> @@ -15,13 +15,12 @@
>>
>> #include <mysql/plugin.h>
>>
>> -typedef struct st_mysql_plugin builtin_plugin[];
>> -
>> -extern builtin_plugin
>> - builtin_binlog_plugin@mysql_plugin_defs@;
>> -
>> -struct st_mysql_plugin *mysqld_builtins[]=
>> +typedef struct st_maria_plugin builtin_maria_plugin[];
>> +
>> +extern builtin_maria_plugin
>> + builtin_maria_binlog_plugin@maria_plugin_defs@;
>> +
>> +struct st_maria_plugin *mariadb_builtins[]=
>> {
>> - builtin_binlog_plugin@mysql_plugin_defs@,(struct st_mysql_plugin
>> *)0
>> + builtin_maria_binlog_plugin@maria_plugin_defs@,(struct
>> st_maria_plugin *)0
>> };
>> -
>
> did you have to rename these structures ?
I thought it would be better as far as they differ from MySQL ones.
>> === modified file 'sql/sql_plugin.cc'
>> --- a/sql/sql_plugin.cc 2009-11-12 04:31:28 +0000
>> +++ b/sql/sql_plugin.cc 2010-03-11 15:02:03 +0000
>> @@ -341,11 +349,261 @@
>> dlclose(p->handle);
>> #endif
>> my_free(p->dl.str, MYF(MY_ALLOW_ZERO_PTR));
>> - if (p->version != MYSQL_PLUGIN_INTERFACE_VERSION)
>> + if (p->mariaversion != MARIA_PLUGIN_INTERFACE_VERSION)
>> my_free((uchar*)p->plugins, MYF(MY_ALLOW_ZERO_PTR));
>> }
>>
>>
>> +/**
>> + Reads data from mysql plugin interface
>> +
>> + @param plugin_dl Structure where the data should be put
>> + @param sym Reverence on version info
>> + @param dlpath Path to the module
>> + @param report What errors should be reported
>> +
>> + @retval FALSE OK
>> + @retval TRUE ERROR
>> +*/
>> +
>> +static my_bool read_mysql_plugin_info(struct st_plugin_dl
>> *plugin_dl,
>> + void *sym, char *dlpath,
>> + int report)
>> +{
>> + DBUG_ENTER("read_maria_plugin_info");
>> + /* Determine interface version */
>> + if (!sym)
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_FIND_DL_ENTRY, MYF(0),
>> plugin_interface_version_sym);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_FIND_DL_ENTRY),
>> plugin_interface_version_sym);
>> + DBUG_RETURN(TRUE);
>
> this needs to be fixed to match the latest code in the pluggable-
> auth branch.
> (where I've backported few bugfixes and small cleanups from mysql-
> next)
>
> in particular
> * don't use my_error()/sql_print_error(), use report_error().
> * fix the bug, mentioned below
I saw your bugfix but left it for merge, usually it is not good to fix
the same things in different paches
>> + }
>> + plugin_dl->mariaversion= 0;
>> + plugin_dl->mysqlversion= *(int *)sym;
>> + /* Versioning */
>> + if (plugin_dl->mysqlversion < min_plugin_interface_version ||
>> + (plugin_dl->mysqlversion >> 8) >
>> (MYSQL_PLUGIN_INTERFACE_VERSION >> 8))
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_OPEN_LIBRARY, MYF(0), dlpath, 0,
>> + "plugin interface version mismatch");
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_OPEN_LIBRARY), dlpath, 0,
>> + "plugin interface version mismatch");
>> + DBUG_RETURN(TRUE);
>> + }
>> + /* Find plugin declarations */
>> + if (!(sym= dlsym(plugin_dl->handle, plugin_declarations_sym)))
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_FIND_DL_ENTRY, MYF(0),
>> plugin_declarations_sym);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_FIND_DL_ENTRY),
>> plugin_declarations_sym);
>> + DBUG_RETURN(TRUE);
>> + }
>> +
>> + /* convert mysql declaration to maria one */
>> + {
>> + int i;
>> + uint sizeof_st_plugin;
>> + struct st_mysql_plugin *old;
>> + struct st_maria_plugin *cur;
>> + char *ptr= (char *)sym;
>> +
>> + if ((sym= dlsym(plugin_dl->handle, sizeof_st_plugin_sym)))
>> + sizeof_st_plugin= *(int *)sym;
>> + else
>> + {
>> +#ifdef ERROR_ON_NO_SIZEOF_PLUGIN_SYMBOL
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_FIND_DL_ENTRY, MYF(0),
>> sizeof_st_plugin_sym);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_FIND_DL_ENTRY),
>> sizeof_st_plugin_sym);
>> + DBUG_RETURN(TRUE);
>> +#else
>> + /*
>> + When the following assert starts failing, we'll have to
>> switch
>> + to the upper branch of the #ifdef
>> + */
>> + DBUG_ASSERT(min_plugin_interface_version == 0);
>> + sizeof_st_plugin= (int)offsetof(struct st_mysql_plugin,
>> version);
>> +#endif
>
> I suppose you can remove this #ifdef now.
I removed it in our function but left code of mysql plugin reading as
is. If you think that it would be better to remove, it is easy.
>> + }
>> +
>> + for (i= 0;
>> + ((struct st_mysql_plugin *)(ptr+i*sizeof_st_plugin))->info;
>> + i++)
>> + /* no op */;
>> +
>> + cur= (struct st_maria_plugin*)
>> + my_malloc(i * sizeof(struct st_maria_plugin),
>
> it's a bug, which I've fixed just recently. See
> http://bazaar.launchpad.net/~maria-captains/maria/5.2-pluggable-auth/revision/2643.11.166
I saw the bug, but left it for merge.
>> + MYF(MY_ZEROFILL|MY_WME));
>> + if (!cur)
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_OUTOFMEMORY, MYF(0), plugin_dl->dl.length);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_OUTOFMEMORY), plugin_dl->dl.length);
>> + DBUG_RETURN(TRUE);
>> + }
>> + /*
>> + All st_plugin fields not initialized in the plugin
>> explicitly, are
>> + set to 0. It matches C standard behaviour for struct
>> initializers that
>> + have less values than the struct definition.
>> + */
>> + for (i=0;
>> + (old=(struct st_mysql_plugin *)(ptr+i*sizeof_st_plugin))-
>> >info;
>> + i++)
>> + {
>> +
>> + cur->type= old->type;
>> + cur->info= old->info;
>> + cur->name= old->name;
>> + cur->author= old->author;
>> + cur->descr= old->descr;
>> + cur->license= old->license;
>> + cur->init= old->init;
>> + cur->deinit= old->deinit;
>> + cur->version= old->version;
>> + cur->status_vars= old->status_vars;
>> + cur->system_vars= old->system_vars;
>> + /*
>> + Something like this should be added to process
>> + new mysql plugin versions:
>> + if (plugin_dl->mysqlversion > 0x0100)
>> + {
>> + cur->newfield= CONSTANT_MEANS_UNKNOWN;
>> + }
>> + else
>> + {
>> + cur->newfield= old->newfield;
>> + }
>> + */
>> + /* Maria only fields */
>> + cur->version_info= "Unknown";
>> + cur->maturity= PLUGIN_MATURITY_UNKNOWN;
>> + }
>> +
>> + plugin_dl->plugins= (struct st_maria_plugin *)cur;
>> + }
>> +
>> + DBUG_RETURN(FALSE);
>> +}
>> +
>> +
>> +/**
>> + Reads data from maria plugin interface
>> +
>> + @param plugin_dl Structure where the data should be put
>> + @param sym Reverence on version info
>> + @param dlpath Path to the module
>> + @param report what errors should be reported
>> +
>> + @retval FALSE OK
>> + @retval TRUE ERROR
>> +*/
>> +
>> +static my_bool read_maria_plugin_info(struct st_plugin_dl
>> *plugin_dl,
>> + void *sym, char *dlpath,
>> + int report)
>> +{
>> + DBUG_ENTER("read_maria_plugin_info");
>> +
>> + /* Determine interface version */
>> + if (!(sym))
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_FIND_DL_ENTRY, MYF(0),
>> plugin_interface_version_sym);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_FIND_DL_ENTRY),
>> plugin_interface_version_sym);
>> + DBUG_RETURN(TRUE);
>> + }
>> + plugin_dl->mariaversion= *(int *)sym;
>> + plugin_dl->mysqlversion= 0;
>> + /* Versioning */
>> + if (plugin_dl->mariaversion < min_maria_plugin_interface_version
>> ||
>> + (plugin_dl->mariaversion >> 8) >
>> (MARIA_PLUGIN_INTERFACE_VERSION >> 8))
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_OPEN_LIBRARY, MYF(0), dlpath, 0,
>> + "plugin interface version mismatch");
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_OPEN_LIBRARY), dlpath, 0,
>> + "plugin interface version mismatch");
>> + DBUG_RETURN(TRUE);
>> + }
>> + /* Find plugin declarations */
>> + if (!(sym= dlsym(plugin_dl->handle,
>> maria_plugin_declarations_sym)))
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_FIND_DL_ENTRY, MYF(0),
>> plugin_declarations_sym);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_FIND_DL_ENTRY),
>> plugin_declarations_sym);
>> + DBUG_RETURN(TRUE);
>> + }
>> + if (plugin_dl->mariaversion != MARIA_PLUGIN_INTERFACE_VERSION)
>> + {
>> + int i;
>> + uint sizeof_st_plugin;
>> + struct st_maria_plugin *old, *cur;
>> + char *ptr= (char *)sym;
>> +
>> + if ((sym= dlsym(plugin_dl->handle, maria_sizeof_st_plugin_sym)))
>> + sizeof_st_plugin= *(int *)sym;
>> + else
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_CANT_FIND_DL_ENTRY, MYF(0),
>> sizeof_st_plugin_sym);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_CANT_FIND_DL_ENTRY),
>> sizeof_st_plugin_sym);
>> + DBUG_RETURN(TRUE);
>> + }
>> +
>> + for (i= 0;
>> + ((struct st_maria_plugin *)(ptr+i*sizeof_st_plugin))->info;
>> + i++)
>> + /* no op */;
>> +
>> + cur= (struct st_maria_plugin*)
>> + my_malloc(i * sizeof(struct st_maria_plugin),
>> + MYF(MY_ZEROFILL|MY_WME));
>> + if (!cur)
>> + {
>> + free_plugin_mem(plugin_dl);
>> + if (report & REPORT_TO_USER)
>> + my_error(ER_OUTOFMEMORY, MYF(0), plugin_dl->dl.length);
>> + if (report & REPORT_TO_LOG)
>> + sql_print_error(ER(ER_OUTOFMEMORY), plugin_dl->dl.length);
>> + DBUG_RETURN(TRUE);
>> + }
>> + /*
>> + All st_plugin fields not initialized in the plugin
>> explicitly, are
>> + set to 0. It matches C standard behaviour for struct
>> initializers that
>> + have less values than the struct definition.
>> + */
>> + for (i=0;
>> + (old=(struct st_maria_plugin *)(ptr+i*sizeof_st_plugin))-
>> >info;
>> + i++)
>> + memcpy(cur+i, old, min(sizeof(cur[i]), sizeof_st_plugin));
>> +
>> + sym= cur;
>> + }
>> + plugin_dl->plugins= (struct st_maria_plugin *)sym;
>> +
>> + DBUG_RETURN(FALSE);
>> +}
>
> I don't know, two almost identical functions - this doesn't look
> very nice.
> can you try to reduce the amount of code duplication ?
The structures are different and could be more different. All ways
which I come up with looked like hacks (like type casting for same
parts of structures) so I decided better to keep it in different
functions.
>> static st_plugin_dl *plugin_dl_add(const LEX_STRING *dl, int report)
>> {
>> #ifdef HAVE_DLOPEN
>> === modified file 'sql/sql_show.cc'
>> --- a/sql/sql_show.cc 2009-11-12 04:31:28 +0000
>> +++ b/sql/sql_show.cc 2010-03-11 15:02:03 +0000
>> @@ -143,7 +151,9 @@
>> table->field[5]->set_notnull();
>> table->field[6]->store(version_buf,
>> make_version_string(version_buf, sizeof(version_buf),
>> - plugin_dl->version),
>> + (plugin_dl->mariaversion ?
>> + plugin_dl->mariaversion :
>> + plugin_dl->mysqlversion)),
>
> okay, although I'd probably just print plugin_dl->mariaversion here.
> if it's 0 - then it's 0. Otherwise the version number cannot be
> interpreted
> unambiguously.
OK
>> cs);
>> table->field[6]->set_notnull();
>> }
>> === modified file 'storage/pbxt/src/ha_pbxt.cc'
>> --- a/storage/pbxt/src/ha_pbxt.cc 2009-09-03 06:15:03 +0000
>> +++ b/storage/pbxt/src/ha_pbxt.cc 2010-03-11 15:02:03 +0000
>> @@ -5507,6 +5507,42 @@
>> drizzle_declare_plugin_end;
>> #else
>> mysql_declare_plugin_end;
>> +#ifdef MARIADB_BASE_VERSION
>> +maria_declare_plugin(pbxt)
>> +{ /* PBXT */
>> + MYSQL_STORAGE_ENGINE_PLUGIN,
>> + &pbxt_storage_engine,
>> + "PBXT",
>> + "Paul McCullagh, PrimeBase Technologies GmbH",
>> + "High performance, multi-versioning transactional engine",
>> + PLUGIN_LICENSE_GPL,
>> + pbxt_init, /* Plugin Init */
>> + pbxt_end, /* Plugin Deinit */
>> + 0x0001 /* 0.1 */,
>> + NULL, /* status variables */
>> + pbxt_system_variables, /* system variables */
>> + "1.0.09g RC3", /* string version */
>> + PLUGIN_MATURITY_GAMMA, /* maturity */
>> + NULL /* config options */
>> +},
>> +{ /* PBXT_STATISTICS */
>> + MYSQL_INFORMATION_SCHEMA_PLUGIN,
>> + &pbxt_statitics,
>> + "PBXT_STATISTICS",
>> + "Paul McCullagh, PrimeBase Technologies GmbH",
>> + "PBXT internal system statitics",
>> + PLUGIN_LICENSE_GPL,
>> + pbxt_init_statitics, /* plugin init */
>> + pbxt_exit_statitics, /* plugin deinit */
>> + 0x0005,
>> + NULL, /* status variables */
>> + NULL, /* system variables */
>> + "1.0.09g RC3", /* string version */
>> + PLUGIN_MATURITY_GAMMA, /* maturity */
>> + NULL /* config options */
>> +}
>> +maria_declare_plugin_end;
>> +#endif
>
> Paul needs to know about this to update his repository
I think letter is enough, right?
>> #endif
>>
>> #if defined(XT_WIN) && defined(XT_COREDUMP)
> Regards,
> Sergei
30 Mar
30 Mar
12:56 p.m.
New subject: [Maria-developers] Rev 2734: Maria WL#61
Hi, Oleksandr! On Mar 29, Oleksandr Byelkin wrote:
+#define PLUGIN_MATURITY_TEST 1 +#define PLUGIN_MATURITY_ALPHA 2 +#define PLUGIN_MATURITY_BETA 3 +#define PLUGIN_MATURITY_GAMMA 4 +#define PLUGIN_MATURITY_RELEASE 5
"release" ? or production or stable ? there can be an alpha release, or beta release. obviously, "release" does not indicate a degree of maturity.
and I don't like "test" either. PLUGIN_MATURITY_TEST is just not readable. I'd rather drop it and start with alpha.
I agree with changing RELEASE with STABLE, but test looks logical for me (and for Monty also as far he invented it).
"maturity test" ? You test somebody's maturity ? :) I think the name is badly chosen and confusing. I my opinion, "alhpa" is enough. or just let them use UNKNOWN for something which is worse than alpha.
@@ -94,15 +112,29 @@
#ifndef MYSQL_DYNAMIC_PLUGIN + #define __MYSQL_DECLARE_PLUGIN(NAME, VERSION, PSIZE, DECLS) \ int VERSION= MYSQL_PLUGIN_INTERFACE_VERSION; \ int PSIZE= sizeof(struct st_mysql_plugin); \ struct st_mysql_plugin DECLS[]= { + +#define __MARIA_DECLARE_PLUGIN(NAME, VERSION, PSIZE, DECLS) \
don't use names that start from __, the C99 standard says "All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use." (ISO/IEC 9899:1999, 7.1.3 Reserved Identifiers)
It's unfortunate that __MYSQL_DECLARE_PLUGIN breaks this rule, but it's not a reason for us to do the same
So what will be better, one or three or remove '_' at all?
three ___ still start from __ :) I meant, just remove leading __. If you want to highligh that the macro is for internal use, you can, for example, add __ at the end, or add _impl or something: MARIA_DECLARE_PLUGIN__ MARIA_DECLARE_PLUGIN_impl
=== modified file 'sql/ha_ndbcluster.cc' --- a/sql/ha_ndbcluster.cc 2009-09-07 20:50:10 +0000 +++ b/sql/ha_ndbcluster.cc 2010-03-11 15:02:03 +0000 @@ -10561,5 +10561,23 @@ NULL /* config options */ } mysql_declare_plugin_end; +maria_declare_plugin(ndbcluster) +{ + MYSQL_STORAGE_ENGINE_PLUGIN, + &ndbcluster_storage_engine, + ndbcluster_hton_name, + "MySQL AB", + "Clustered, fault-tolerant tables", + PLUGIN_LICENSE_GPL, + ndbcluster_init, /* Plugin Init */ + NULL, /* Plugin Deinit */ + 0x0100 /* 1.0 */, + ndb_status_variables_export,/* status variables */ + NULL, /* system variables */ + "1.0", /* string version */ + PLUGIN_MATURITY_BETA, /* maturity */
beta ? says who ?
My guess reviewed by Monty.
let's call it gamma (discussed on IRC)
+ NULL /* config options */ +} +maria_declare_plugin_end; === modified file 'sql/sql_plugin.cc' --- a/sql/sql_plugin.cc 2009-11-12 04:31:28 +0000 +++ b/sql/sql_plugin.cc 2010-03-11 15:02:03 +0000 ..... I saw your bugfix but left it for merge, usually it is not good to fix the same things in different paches
I just mean that during the merge you need to remember to fix them in your copy-pasted function, bzr won't do it automatically.
=== modified file 'storage/pbxt/src/ha_pbxt.cc' --- a/storage/pbxt/src/ha_pbxt.cc 2009-09-03 06:15:03 +0000 +++ b/storage/pbxt/src/ha_pbxt.cc 2010-03-11 15:02:03 +0000 ... +{ /* PBXT_STATISTICS */ + MYSQL_INFORMATION_SCHEMA_PLUGIN, + &pbxt_statitics, + "PBXT_STATISTICS", + "Paul McCullagh, PrimeBase Technologies GmbH", + "PBXT internal system statitics", + PLUGIN_LICENSE_GPL, + pbxt_init_statitics, /* plugin init */ + pbxt_exit_statitics, /* plugin deinit */ + 0x0005, + NULL, /* status variables */ + NULL, /* system variables */ + "1.0.09g RC3", /* string version */ + PLUGIN_MATURITY_GAMMA, /* maturity */ + NULL /* config options */ +} +maria_declare_plugin_end; +#endif
Paul needs to know about this to update his repository
I think letter is enough, right?
Yes. Regards, Sergei
5343
Age (days ago)
5344
Last active (days ago)
1 comments
2 participants
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik