Hi Tor,
On 12/15/10 6:32 AM, Tor Didriksen wrote:
> #At file:///export/home/didrik/repo/5.5-bugteam-bug58137/ based on
> revid:gleb.shchepa@stripped
>
> 3195 Tor Didriksen 2010-12-13
> Bug #58699 cannot build with gcc dbg on solaris
> @ include/my_pthread.h
> On solaris pthread_once_t contains an array, so we must wrap the
> initialization in {}
> @ include/mysql/psi/mysql_file.h
> Include my_global.h first, to get correct platform definitions.
> @ mysys/ptr_cmp.c
> Hide the unused static functions in #ifdef's on solaris.
> @ sql/my_decimal.cc
> Include my_global.h first, to get correct platform definitions.
> @ sql/mysqld.cc
> Fix signed/unsigned comparison warning.
> @ sql/sql_audit.h
> Include my_global.h first, to get correct platform definitions.
> @ sql/sql_plugin.h
> Include my_global.h first, to get correct platform definitions.
> @ sql/sql_show.cc
> Fix: warning: cast from pointer to integer of different size
> @ sql/sys_vars.h
> Use reinterpret_cast rather than c-style cast.
> @ storage/perfschema/pfs_instr.cc
> Include my_global.h first, to get correct platform definitions.
Some comments below.
> modified:
> include/my_pthread.h
> include/mysql/psi/mysql_file.h
> mysys/ptr_cmp.c
> sql/my_decimal.cc
> sql/mysqld.cc
> sql/sql_audit.h
> sql/sql_plugin.h
> sql/sql_show.cc
> sql/sys_vars.h
> storage/perfschema/pfs_instr.cc
> === modified file 'include/my_pthread.h'
> --- a/include/my_pthread.h 2010-10-20 14:48:19 +0000
> +++ b/include/my_pthread.h 2010-12-13 16:04:03 +0000
> @@ -214,7 +214,11 @@ int pthread_cancel(pthread_t thread);
> typedef void *(* pthread_handler)(void *);
>
> #define my_pthread_once_t pthread_once_t
> +#if defined __GNUC__ && defined TARGET_OS_SOLARIS
defined(__GNUC__) && defined(TARGET_OS_SOLARIS) as used in other places
in this file. I believe you told me something very similar in a review
this week :-)
> +#define MY_PTHREAD_ONCE_INIT { PTHREAD_ONCE_INIT }
Please reference, in the per-file comment, the bug report about this:
http://bugs.opensolaris.org/bugdatabase/printableBug.do?bug_id=6611808
Also, it seems to me that this is something we should detect at cmake
time, or tie it to a set of specific Solaris versions (8 and 9). A test
at cmake time for Solaris 8 and 9 to ensure that { PTHREAD_ONCE_INIT }
compiles is one option.
> +#else
> #define MY_PTHREAD_ONCE_INIT PTHREAD_ONCE_INIT
> +#endif
> #define my_pthread_once(C,F) pthread_once(C,F)
>
> /* Test first for RTS or FSU threads */
>
> === modified file 'include/mysql/psi/mysql_file.h'
> --- a/include/mysql/psi/mysql_file.h 2010-07-09 23:00:24 +0000
> +++ b/include/mysql/psi/mysql_file.h 2010-12-13 16:04:03 +0000
> @@ -16,6 +16,8 @@
> #ifndef MYSQL_FILE_H
> #define MYSQL_FILE_H
>
> +#include "my_global.h"
<my_global.h>
Just a style preference in line with the other includes in the file. I
wonder if there are any practical implications for MySQL code (other
then the normal difference).
> +
> /* For strlen() */
> #include<string.h>
> /* For MY_STAT */
>
> === modified file 'mysys/ptr_cmp.c'
> --- a/mysys/ptr_cmp.c 2009-11-20 19:01:43 +0000
> +++ b/mysys/ptr_cmp.c 2010-12-13 16:04:03 +0000
> @@ -50,11 +50,15 @@ static int ptr_compare_3(size_t *compare
>
> /* Get a pointer to a optimal byte-compare function for a given size */
>
> +#ifdef TARGET_OS_SOLARIS
> qsort2_cmp get_ptr_compare (size_t size)
> {
> -#ifdef TARGET_OS_SOLARIS
> + (void) size;
Use the unused attribute tag. It can be used even when the argument is
being used.
Also, let's get rid of TARGET_OS_SOLARIS? It's only used in a few places
and we should actually be using a macro provided by Solaris.
> return (qsort2_cmp) native_compare;
> +}
> #else
> +qsort2_cmp get_ptr_compare (size_t size)
> +{
> if (size< 4)
> return (qsort2_cmp) ptr_compare;
> switch (size& 3) {
> @@ -64,8 +68,8 @@ qsort2_cmp get_ptr_compare (size_t size)
> case 3: return (qsort2_cmp) ptr_compare_3;
> }
> return 0; /* Impossible */
> -#endif /* TARGET_OS_SOLARIS */
> }
> +#endif /* TARGET_OS_SOLARIS */
>
>
> /*
> @@ -75,6 +79,8 @@ qsort2_cmp get_ptr_compare (size_t size)
>
> #define cmp(N) if (first[N] != last[N]) return (int) first[N] - (int) last[N]
>
> +#ifndef TARGET_OS_SOLARIS
> +
> static int ptr_compare(size_t *compare_length, uchar **a, uchar **b)
> {
> reg3 int length= *compare_length;
> @@ -177,6 +183,8 @@ static int ptr_compare_3(size_t *compare
> return (0);
> }
>
> +#endif /* !TARGET_OS_SOLARIS */
> +
> void my_store_ptr(uchar *buff, size_t pack_length, my_off_t pos)
> {
> switch (pack_length) {
>
> === modified file 'sql/my_decimal.cc'
> --- a/sql/my_decimal.cc 2010-12-14 16:26:18 +0000
> +++ b/sql/my_decimal.cc 2010-12-13 16:04:03 +0000
> @@ -13,6 +13,7 @@
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
>
> +#include "my_global.h"
> #include "sql_priv.h"
> #include<time.h>
>
>
> === modified file 'sql/mysqld.cc'
> --- a/sql/mysqld.cc 2010-12-14 14:34:23 +0000
> +++ b/sql/mysqld.cc 2010-12-13 16:04:03 +0000
> @@ -3258,8 +3258,8 @@ static int init_common_variables()
> size_t *pagesize = (size_t *) malloc(sizeof(size_t) * nelem);
> if (pagesize != NULL&& getpagesizes(pagesize, nelem)> 0)
> {
> - size_t i, max_page_size= 0;
> - for (i= 0; i< nelem; i++)
> + size_t max_page_size= 0;
> + for (int i= 0; i< nelem; i++)
This is prohibited in the CodingStyle because some of old compilers in
platforms we support for 5.1. Need to double check if we support those
platforms for 5.5. I also vaguely remember that this might have been
changed in some CodingStyle meeting.. but I'm not sure.
> {
> if (pagesize[i]> max_page_size&&
> pagesize[i]<= max_desired_page_size)
>
> === modified file 'sql/sql_audit.h'
> --- a/sql/sql_audit.h 2010-12-14 14:34:23 +0000
> +++ b/sql/sql_audit.h 2010-12-13 16:04:03 +0000
> @@ -17,6 +17,8 @@
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
>
>
> +#include "my_global.h"
> +
> #include<mysql/plugin_audit.h>
> #include "sql_class.h"
>
>
> === modified file 'sql/sql_plugin.h'
> --- a/sql/sql_plugin.h 2010-12-01 22:15:14 +0000
> +++ b/sql/sql_plugin.h 2010-12-13 16:04:03 +0000
> @@ -16,6 +16,8 @@
> #ifndef _sql_plugin_h
> #define _sql_plugin_h
>
> +#include "my_global.h"
> +
> /*
> the following #define adds server-only members to enum_mysql_show_type,
> that is defined in plugin.h
>
> === modified file 'sql/sql_show.cc'
> --- a/sql/sql_show.cc 2010-12-14 10:46:00 +0000
> +++ b/sql/sql_show.cc 2010-12-13 16:04:03 +0000
> @@ -2246,7 +2246,7 @@ static bool show_status_array(THD *thd,
> end= int10_to_str(*(long*) value, buff, 10);
> break;
> case SHOW_LONGLONG_STATUS:
> - value= ((char *) status_var + (ulonglong) value);
> + value= ((char *) status_var + (ulong) value);
> /* fall through */
> case SHOW_LONGLONG:
> end= longlong10_to_str(*(longlong*) value, buff, 10);
>
Otherwise, looks good.
Regards,
Davi
