Hi, Vladislav! On Mar 14, Vladislav Vaintroub wrote:
Hi, this broke Windows build. Would it be possible to fix it?
When looking at the patch, I found some things that IMO could be done better.
1. mysys/my_logger.c
extern char *mysql_data_home; extern PSI_mutex_key key_LOCK_logger_service;
I think referencing variables that do not belong to mysys in mysys is a layering violation. Can mysys be used without sql or not here. Shall everything that links to mysys link sql as well?
I think - If there is a need to store data home and PSI key from sql layer, then this something can be passed via initialization function, rather than used directly. Alternatively: Is the functionality provided by my_logger required in mysys? If not, why it is be there, and can it be moved to where it is used, e.g into sql?
Right. As the logger is in mysys, the psi initialization of key_LOCK_logger_service should be in mysys. As for mysql_data_home - it's not needed at all here, I suspect. Plugins are initialized after the server has chdir-ed into mysql_data_home. So here we can use current directory and relative paths instead.
2. LOGGER_HANDLE *logger_open(const char *path, unsigned long size_limit, unsigned int rotations)
Is (general case) 32 bit size limit limitation by design ? IF so, why is not "int" used (or int32 if one wants a fancy typename)? Files can be larger than 4GB. Even on 32 bits, and on 64 bit systems with 32 bit longs.
Can this be fixed?
ulonglong would be more appropriate, indeed Regards, Sergei