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

[Monty Taylor]

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

Well... you are right, I thougt my_global.h was getting included from
somewhere, but now I'm not seeing the problem anymore... so I'll remove
those from the patch.

> my_global.h/mysql_com.h:
> 1) neither of these are public headers

They are installed. They are included by mysql.h. They are public.

I agree though... they _should_not_ be public. But that's a much larger
cleanup that is not for us. :)

> 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.

I'll go find a MySQL server fellow and send in that part to him directly.

> 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)

Yup. Fair enough.
> 
> 
> 
> 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...

Yes. They are defined twice.