maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

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