Re: bzr commit into mysql-5.1-telco-6.2 branch (monty:2633) Bug#38177

[Jonas Oreland]

summary: - some questions on ndb inline below
         - the fact that you modify my_global.h still makes me believe that you do
something wrong

my_global.h/mysql_com.h:
1) neither of these are public headers
2) you told me you (illegally) use these for decimal etc conversion, right?
3) but that does not (in my book) imply that they should be exposed in "Public headers"
   i.e you can (if someone looks the other way) use them in your .c-files.
       but using them in .h-files sounds *insane*
4) it could also be that your unspecified test-program include them directly.
   in that case, you'll have to try to get a "sql-server" guy to review them instead
   and commit it in separate patch.

Why am I so resistant to your changes (in mysql-server part)?
- cause there is *0* benefit for telco-trees getting the change above (headers are private
anyway)
- cause there is a potential merge conflict coming up
- cause the unwritten rule is that we try not to make changes to server wo/ getting
reviews from them
  (the same way we don't expect them to make changes to our code wo/ getting review)




Monty Taylor wrote:
> #At http://bazaar.launchpad.net/~mordred/mysql-server/5.1-telco-6.2
> 
>  2633 Monty Taylor	2008-08-05
>       BUG#38177 - Public headers prevent applications from being built with warnings
>       turned on
>             
>       Fixed all of the build warnings produced by compiling a test program with: "-W
> -Wall -Wextra -std=gnu++98 -pedantic -Wundef -Wredundant-decls -Wno-long-long
> -Wno-strict-aliasing  -Werror"
> modified:
>   include/my_global.h
>   include/mysql_com.h
>   storage/ndb/include/mgmapi/mgmapi.h
>   storage/ndb/include/ndbapi/NdbIndexScanOperation.hpp
>   storage/ndb/include/ndbapi/NdbOperation.hpp
>   storage/ndb/include/ndbapi/NdbTransaction.hpp
> 
> === modified file 'include/my_global.h'
> --- a/include/my_global.h	2008-03-28 10:14:27 +0000
> +++ b/include/my_global.h	2008-08-05 21:16:53 +0000
> @@ -235,6 +235,7 @@
>    now let's figure out if inline functions are supported
>    autoconf defines 'inline' to be empty, if not
>  */
> +#define inline1 0
>  #define inline_test_1(X)        X ## 1
>  #define inline_test_2(X)        inline_test_1(X)
>  #if inline_test_2(inline) != 1
> @@ -242,6 +243,7 @@
>  #endif
>  #undef inline_test_2
>  #undef inline_test_1
> +#undef inline1
>  /* helper macro for "instantiating" inline functions */
>  #define STATIC_INLINE static inline
>  
> 
> === modified file 'include/mysql_com.h'
> --- a/include/mysql_com.h	2008-04-08 08:31:46 +0000
> +++ b/include/mysql_com.h	2008-08-05 21:16:53 +0000
> @@ -486,9 +486,6 @@ const char *mysql_errno_to_sqlstate(unsi
>  
>  /* Some other useful functions */
>  
> -my_bool my_thread_init(void);
> -void my_thread_end(void);
> -
>  #ifdef _global_h
>  ulong STDCALL net_field_length(uchar **packet);
>  my_ulonglong net_field_length_ll(uchar **packet);
> 

> === modified file 'storage/ndb/include/mgmapi/mgmapi.h'
> --- a/storage/ndb/include/mgmapi/mgmapi.h	2008-03-28 10:31:07 +0000
> +++ b/storage/ndb/include/mgmapi/mgmapi.h	2008-08-05 21:16:53 +0000
> @@ -443,11 +443,7 @@ extern "C" {
>     */
>    int ndb_mgm_number_of_mgmd_in_connect_string(NdbMgmHandle handle);
>  
> -  int ndb_mgm_set_configuration_nodeid(NdbMgmHandle handle, int nodeid);
>    int ndb_mgm_get_configuration_nodeid(NdbMgmHandle handle);
> -  int ndb_mgm_get_connected_port(NdbMgmHandle handle);
> -  const char *ndb_mgm_get_connected_host(NdbMgmHandle handle);
> -  const char *ndb_mgm_get_connectstring(NdbMgmHandle handle, char *buf, int
> buf_sz);

are these defined twice ?
or else why remove them...