From 807e4f320fe5e4531fbc178552b8c30f09a7d2df Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 10 Dec 2024 18:00:37 +1100 Subject: [PATCH] Change my_umask{,_dir} to mode_t and remove os_innodb_umask MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit os_innodb_umask was of the incorrect type resulting in warnings in clang-19. The correct type is mode_t. As os_innodb_umask was set during innnodb_init from my_umask, corrected the type there along with its companion my_umask_dir. Because of this, the defaults mask values in innodb never had an effect. The resulting change allow found signed differences in my_create{,_nosymlink}, open_nosymlinks: mysys/my_create.c:47:20: error: operand of ?: changes signedness from ‘int’ to ‘mode_t’ {aka ‘unsigned int’} due to unsignedness of other operand [-Werror=sign-compare] 47 | CreateFlags ? CreateFlags : my_umask); Ref: clang-19 warnings: [55/123] Building CXX object storage/innobase/CMakeFiles/innobase.dir/os/os0file.cc.o storage/innobase/os/os0file.cc:1075:46: warning: implicit conversion loses integer precision: 'ulint' (aka 'unsigned long') to 'mode_t' (aka 'unsigned int') [-Wshorten-64-to-32] 1075 | file = open(name, create_flag | O_CLOEXEC, os_innodb_umask); | ~~~~ ^~~~~~~~~~~~~~~ storage/innobase/os/os0file.cc:1249:46: warning: implicit conversion loses integer precision: 'ulint' (aka 'unsigned long') to 'mode_t' (aka 'unsigned int') [-Wshorten-64-to-32] 1249 | file = open(name, create_flag | O_CLOEXEC, os_innodb_umask); | ~~~~ ^~~~~~~~~~~~~~~ storage/innobase/os/os0file.cc:1381:45: warning: implicit conversion loses integer precision: 'ulint' (aka 'unsigned long') to 'mode_t' (aka 'unsigned int') [-Wshorten-64-to-32] 1381 | file = open(name, create_flag | O_CLOEXEC, os_innodb_umask); | ~~~~ ^~~~~~~~~~~~~~~ --- include/my_sys.h | 10 +++++----- mysys/my_create.c | 4 ++-- mysys/my_init.c | 8 ++++---- mysys/my_open.c | 2 +- mysys/my_static.c | 2 +- mysys/my_symlink2.c | 2 +- storage/innobase/handler/ha_innodb.cc | 2 -- storage/innobase/include/os0file.h | 5 ----- storage/innobase/os/os0file.cc | 28 +++------------------------ 9 files changed, 17 insertions(+), 46 deletions(-) diff --git a/include/my_sys.h b/include/my_sys.h index d1a0394086c..856640a3a90 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -246,9 +246,9 @@ extern void (*my_sigtstp_cleanup)(void), /* Executed before jump to shell */ (*my_sigtstp_restart)(void); /* Executed when coming from shell */ -extern MYSQL_PLUGIN_IMPORT int my_umask; /* Default creation mask */ -extern int my_umask_dir, - my_recived_signals, /* Signals we have got */ +extern MYSQL_PLUGIN_IMPORT mode_t my_umask; /* Default creation mask */ +extern mode_t my_umask_dir; +extern int my_recived_signals, /* Signals we have got */ my_safe_to_handle_signal, /* Set when allowed to SIGTSTP */ my_dont_interrupt; /* call remember_intr when set */ extern MYSQL_PLUGIN_IMPORT my_bool my_use_symdir; @@ -609,7 +609,7 @@ extern File my_open(const char *FileName,int Flags,myf MyFlags); extern File my_register_filename(File fd, const char *FileName, enum file_type type_of_file, uint error_message_number, myf MyFlags); -extern File my_create(const char *FileName,int CreateFlags, +extern File my_create(const char *FileName, mode_t CreateFlags, int AccessFlags, myf MyFlags); extern int my_close(File Filedes,myf MyFlags); extern int my_mkdir(const char *dir, int Flags, myf MyFlags); @@ -617,7 +617,7 @@ extern int my_readlink(char *to, const char *filename, myf MyFlags); extern int my_is_symlink(const char *filename); extern int my_realpath(char *to, const char *filename, myf MyFlags); extern File my_create_with_symlink(const char *linkname, const char *filename, - int createflags, int access_flags, + mode_t createflags, int access_flags, myf MyFlags); extern int my_rename_with_symlink(const char *from,const char *to,myf MyFlags); extern int my_symlink(const char *content, const char *linkname, myf MyFlags); diff --git a/mysys/my_create.c b/mysys/my_create.c index 6fb817da8df..32cc73a53c4 100644 --- a/mysys/my_create.c +++ b/mysys/my_create.c @@ -33,12 +33,12 @@ */ -File my_create(const char *FileName, int CreateFlags, int access_flags, +File my_create(const char *FileName, mode_t CreateFlags, int access_flags, myf MyFlags) { int fd; DBUG_ENTER("my_create"); - DBUG_PRINT("my",("Name: '%s' CreateFlags: %d AccessFlags: %d MyFlags: %lu", + DBUG_PRINT("my",("Name: '%s' CreateFlags: %u AccessFlags: %d MyFlags: %lu", FileName, CreateFlags, access_flags, MyFlags)); #if defined(_WIN32) fd= my_win_open(FileName, access_flags | O_CREAT); diff --git a/mysys/my_init.c b/mysys/my_init.c index 2b420da03be..fcb3ccfdf31 100644 --- a/mysys/my_init.c +++ b/mysys/my_init.c @@ -45,7 +45,7 @@ uint mysys_usage_id= 0; /* Incremented for each my_init() */ ulonglong my_thread_stack_size= (sizeof(void*) <= 4)? 65536: ((256-16)*1024); -static ulong atoi_octal(const char *str) +static mode_t atoi_octal(const char *str) { long int tmp; while (*str && my_isspace(&my_charset_latin1, *str)) @@ -53,7 +53,7 @@ static ulong atoi_octal(const char *str) str2int(str, (*str == '0' ? 8 : 10), /* Octalt or decimalt */ 0, INT_MAX, &tmp); - return (ulong) tmp; + return (mode_t) tmp; } MYSQL_FILE *mysql_stdin= NULL; @@ -82,10 +82,10 @@ my_bool my_init(void) /* Default creation of new files */ if ((str= getenv("UMASK")) != 0) - my_umask= (int) (atoi_octal(str) | 0600); + my_umask= atoi_octal(str) | 0600; /* Default creation of new dir's */ if ((str= getenv("UMASK_DIR")) != 0) - my_umask_dir= (int) (atoi_octal(str) | 0700); + my_umask_dir= atoi_octal(str) | 0700; init_glob_errs(); diff --git a/mysys/my_open.c b/mysys/my_open.c index 8b5f4f9435e..c39539342d9 100644 --- a/mysys/my_open.c +++ b/mysys/my_open.c @@ -20,7 +20,7 @@ #include "my_atomic.h" CREATE_NOSYMLINK_FUNCTION( - open_nosymlinks(const char *pathname, int flags, int mode), + open_nosymlinks(const char *pathname, int flags, mode_t mode), openat(dfd, filename, O_NOFOLLOW | flags, mode), open(pathname, O_NOFOLLOW | flags, mode) ); diff --git a/mysys/my_static.c b/mysys/my_static.c index 6c09ab8d94b..1e54c4c93b2 100644 --- a/mysys/my_static.c +++ b/mysys/my_static.c @@ -63,7 +63,7 @@ char curr_dir[FN_REFLEN]= {0}, home_dir_buff[FN_REFLEN]= {0}; ulong my_stream_opened=0,my_tmp_file_created=0; ulong my_file_total_opened= 0; -int my_umask=0664, my_umask_dir=0777; +mode_t my_umask=0664, my_umask_dir=0777; myf my_global_flags= 0; #ifndef DBUG_OFF diff --git a/mysys/my_symlink2.c b/mysys/my_symlink2.c index 0b580ecd32f..d9b31e3dfaf 100644 --- a/mysys/my_symlink2.c +++ b/mysys/my_symlink2.c @@ -26,7 +26,7 @@ #include File my_create_with_symlink(const char *linkname, const char *filename, - int createflags, int access_flags, myf MyFlags) + mode_t createflags, int access_flags, myf MyFlags) { File file; int tmp_errno; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 68d486b9457..ad0f894f924 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4024,8 +4024,6 @@ static int innodb_init(void* p) test_filename)); #endif /* DBUG_OFF */ - os_file_set_umask(my_umask); - /* Setup the memory alloc/free tracing mechanisms before calling any functions that could possibly allocate memory. */ ut_new_boot(); diff --git a/storage/innobase/include/os0file.h b/storage/innobase/include/os0file.h index 43320515fbd..e3dc5d16c8e 100644 --- a/storage/innobase/include/os0file.h +++ b/storage/innobase/include/os0file.h @@ -1135,11 +1135,6 @@ os_file_get_status( bool check_rw_perm, bool read_only); -/** Set the file create umask -@param[in] umask The umask to use for file creation. */ -void -os_file_set_umask(ulint umask); - #ifdef _WIN32 /** diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index f33797305dc..012f23b3b8d 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -140,18 +140,6 @@ static io_slots *write_slots; /** Number of retries for partial I/O's */ constexpr ulint NUM_RETRIES_ON_PARTIAL_IO = 10; -/* This specifies the file permissions InnoDB uses when it creates files in -Unix; the value of os_innodb_umask is initialized in ha_innodb.cc to -my_umask */ - -#ifndef _WIN32 -/** Umask for creating files */ -static ulint os_innodb_umask = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP; -#else -/** Umask for creating files */ -static ulint os_innodb_umask = 0; -#endif /* _WIN32 */ - Atomic_counter os_n_file_reads; static ulint os_bytes_read_since_printout; ulint os_n_file_writes; @@ -1072,7 +1060,7 @@ os_file_create_simple_func( bool retry; do { - file = open(name, create_flag | O_CLOEXEC, os_innodb_umask); + file = open(name, create_flag | O_CLOEXEC, my_umask); if (file == -1) { *success = false; @@ -1246,7 +1234,7 @@ os_file_create_func( bool retry; do { - file = open(name, create_flag | O_CLOEXEC, os_innodb_umask); + file = open(name, create_flag | O_CLOEXEC, my_umask); if (file == -1) { const char* operation; @@ -1378,7 +1366,7 @@ os_file_create_simple_no_error_handling_func( return(OS_FILE_CLOSED); } - file = open(name, create_flag | O_CLOEXEC, os_innodb_umask); + file = open(name, create_flag | O_CLOEXEC, my_umask); *success = (file != -1); @@ -3861,16 +3849,6 @@ os_aio_refresh_stats() os_last_printout = time(NULL); } - -/** -Set the file create umask -@param[in] umask The umask to use for file creation. */ -void -os_file_set_umask(ulint umask) -{ - os_innodb_umask = umask; -} - #ifdef _WIN32 /* Checks whether physical drive is on SSD.*/