List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:November 28 2008 5:32pm
Subject:RE: bzr commit into mysql-6.0-bugteam branch (holyfoot:2759) Bug#41103
View as plain text  
The patch is ok as quick fix, and feel free to push it , 

but.. (pardon my short rant below)

Note that you're patching the patch for Bug#38293 that was overly intrusive
and did not link for the first couple of days.

Basically, the original Bug#38293 problem could have been fixed without
adding more code but instead by removing code

-#if defined(USE_TLS)
-  /* This can only happen in a .DLL */
-  if (!tmp)
-  {
-    my_thread_init();
-    tmp=my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
-  }
-#endif

from _my_thread_var.

Instead, an impressive number of internal functions that do not belong to
MySQL API (and do not belong to exported interface of libmysqld.dll) were
added to the exports. I
Some data was added too, hence the described problem. 

I'm not convinced that libmysqld.dll shall export all mysys dbug and string
functionality. libmysqld.dll also has the whole C runtime inside, but this
does not mean user libmysqld.dll shall export C runtime functionality e.g
malloc () and user must not link with libcmt or msvcrt.

I believe, libmysqld.dll should export exactly the same what  mysql client
exports and this is the official api. If a program uses both my_server_init
and my_open, it should link to both libmysqld.dll and mysys.lib - because
the first function is the official api of libmysqld and the second of mysys.
If one does not like it, he could take the static library and link with it.

I understand ,people who are using Unix linkers for are years this position
could be strange on the first glance, but from the clean API-perspective a
shared library should not export more that documented.

All in all, for the future I propose to remove all superfluous exports from
libmysqld.def and also MY_DLLEXPORT that you just introduced...
It would be a whole lot simpler to maintain and build and understand.
And yes, having 2 instances of mysys, dbug and string libraries within a
single process for our test utilities will be necessary (so what ;))
It is already like this today, for any process that is using shared client
library and mysys or whatever internal library.

Just my 2 c.

wlad

> -----Original Message-----
> From: Alexey Botchkov [mailto:holyfoot@stripped]
> Sent: Friday, November 28, 2008 3:53 PM
> To: commits@stripped
> Subject: bzr commit into mysql-6.0-bugteam branch (holyfoot:2759)
> Bug#41103
> 
> #At file:///home/hf/work/mysql_common/dst/
> 
>  2759 Alexey Botchkov	2008-11-28
>       Bug#41103 6.0 Windows embedded-server tests fail
>           to be exported correctly from the DLL, data
>           has to be declared as __declspec(dllimport)
>           for the client, and as DATA in the .def file.
> 
>       per-file comments:
>         include/m_string.h
>       Bug#41103 6.0 Windows embedded-server tests fail
>           __declspec(dllimport) added for Windows
>         include/my_global.h
>       Bug#41103 6.0 Windows embedded-server tests fail
>           __declspec(dllimport) added for Windows
>         include/my_sys.h
>       Bug#41103 6.0 Windows embedded-server tests fail
>           __declspec(dllimport) added for Windows
>         libmysql/libmysql.def
>       Bug#41103 6.0 Windows embedded-server tests fail
>          DATA modifiers added
>         libmysqld/examples/CMakeLists.txt
>       Bug#41103 6.0 Windows embedded-server tests fail
>          MY_USE_CLIENT_DLL defined for projects
>         libmysqld/libmysqld.def
>       Bug#41103 6.0 Windows embedded-server tests fail
>          DATA modifiers added
> modified:
>   include/m_string.h
>   include/my_global.h
>   include/my_sys.h
>   libmysql/libmysql.def
>   libmysqld/examples/CMakeLists.txt
>   libmysqld/libmysqld.def
> 
> === modified file 'include/m_string.h'
> --- a/include/m_string.h	2008-06-26 18:29:30 +0000
> +++ b/include/m_string.h	2008-11-28 14:50:36 +0000
> @@ -89,8 +89,8 @@ extern char *stpcpy(char *, const char *
>  #endif
> 
>  /* Declared in int2str() */
> -extern char NEAR _dig_vec_upper[];
> -extern char NEAR _dig_vec_lower[];
> +MY_DLLEXPORT char NEAR _dig_vec_upper[];
> +MY_DLLEXPORT char NEAR _dig_vec_lower[];
> 
>  #ifdef BAD_STRING_COMPILER
>  #define strmov(A,B)  (memccpy(A,B,0,INT_MAX)-1)
> 
> === modified file 'include/my_global.h'
> --- a/include/my_global.h	2008-11-19 10:04:44 +0000
> +++ b/include/my_global.h	2008-11-28 14:50:36 +0000
> @@ -1615,4 +1615,10 @@ inline void  operator delete[](void*, vo
>  #  define __func__ "<unknown>"
>  #endif
> 
> +#ifdef MY_USE_CLIENT_DLL
> +#define MY_DLLEXPORT __declspec(dllimport)
> +#else
> +#define MY_DLLEXPORT extern
> +#endif /*MY_USE_CLIENT_DLL*/
> +
>  #endif /* my_global_h */
> 
> === modified file 'include/my_sys.h'
> --- a/include/my_sys.h	2008-10-20 19:13:22 +0000
> +++ b/include/my_sys.h	2008-11-28 14:50:36 +0000
> @@ -218,7 +218,7 @@ extern int errno;			/* declare errno
> */
>  #endif					/* #ifndef errno */
>  extern char NEAR errbuff[NRERRBUFFS][ERRMSGSIZE];
>  extern char *home_dir;			/* Home directory for user
*/
> -extern const char *my_progname;		/* program-name (printed in
> errors) */
> +MY_DLLEXPORT const char *my_progname;		/* program-name
> (printed in errors) */
>  extern const char *my_progname_short;	/* like above but without
> directory */
>  extern char NEAR curr_dir[];		/* Current directory for user */
>  extern void (*error_handler_hook)(uint my_err, const char *str,myf
> MyFlags);
> 
> === modified file 'libmysql/libmysql.def'
> --- a/libmysql/libmysql.def	2008-08-11 10:03:45 +0000
> +++ b/libmysql/libmysql.def	2008-11-28 14:50:36 +0000
> @@ -1,8 +1,9 @@
>  LIBRARY		LIBMYSQL
>  VERSION		6.0
>  EXPORTS
> -	_dig_vec_lower
> -	_dig_vec_upper
> +	_dig_vec_lower DATA
> +	_dig_vec_upper DATA
> +        my_progname DATA
>  	bmove_upp
>  	delete_dynamic
>  	free_defaults
> 
> === modified file 'libmysqld/examples/CMakeLists.txt'
> --- a/libmysqld/examples/CMakeLists.txt	2008-11-19 12:11:30 +0000
> +++ b/libmysqld/examples/CMakeLists.txt	2008-11-28 14:50:36 +0000
> @@ -24,7 +24,7 @@ IF(WIN32)
>    ADD_DEFINITIONS(-DUSE_TLS)
>  ENDIF(WIN32)
> 
> -ADD_DEFINITIONS(-DEMBEDDED_LIBRARY)
> +ADD_DEFINITIONS(-DMY_USE_CLIENT_DLL)
> 
>  ADD_EXECUTABLE(mysql_embedded ../../client/completion_hash.cc
>                 ../../client/mysql.cc ../../client/readline.cc
> 
> === modified file 'libmysqld/libmysqld.def'
> --- a/libmysqld/libmysqld.def	2008-11-26 11:54:39 +0000
> +++ b/libmysqld/libmysqld.def	2008-11-28 14:50:36 +0000
> @@ -63,8 +63,8 @@ EXPORTS
>  	my_dir
>  	my_micro_time
>  	find_type_or_exit
> -	_dig_vec_upper
> -	_dig_vec_lower
> +	_dig_vec_upper DATA
> +	_dig_vec_lower DATA
>  	bmove_upp
>  	delete_dynamic
>  	free_defaults
> @@ -179,7 +179,7 @@ EXPORTS
>  	disabled_my_option
>  	my_charset_latin1
>  	init_alloc_root
> -	my_progname
> +	my_progname DATA
>  	get_charset_name
>  	get_charset_by_csname
>  	print_defaults
> 
> 
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1


Thread
bzr commit into mysql-6.0-bugteam branch (holyfoot:2759) Bug#41103Alexey Botchkov28 Nov
  • RE: bzr commit into mysql-6.0-bugteam branch (holyfoot:2759) Bug#41103Vladislav Vaintroub28 Nov