[PATCH v2 0/3] MR5356: server: Replace sprintf with snprintf/asprintf to avoid deprecation warnings on macOS.
-- v2: server: Clarify that registry files are always in the current directory, and simplify save_branch(). server: Replace sprintf with snprintf to avoid deprecation warnings on macOS. server: Replace some malloc/sprintf/strcpy calls with asprintf. https://gitlab.winehq.org/wine/wine/-/merge_requests/5356
From: Brendan Shanks <bshanks(a)codeweavers.com> --- server/request.c | 24 ++++++++++-------------- server/unicode.c | 10 ++-------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/server/request.c b/server/request.c index 8fe3a09cb7e..2691e0c7cff 100644 --- a/server/request.c +++ b/server/request.c @@ -601,9 +601,8 @@ static void create_dir( const char *name, struct stat *st ) static char *create_server_dir( int force ) { const char *prefix = getenv( "WINEPREFIX" ); - char *p, *config_dir; + char *p, *config_dir, *base_dir; struct stat st, st2; - size_t len = sizeof("/server-") + 2 * sizeof(st.st_dev) + 2 * sizeof(st.st_ino) + 2; /* open the configuration directory */ @@ -646,23 +645,19 @@ static char *create_server_dir( int force ) /* create the base directory if needed */ #ifdef __ANDROID__ /* there's no /tmp dir on Android */ - len += strlen( config_dir ) + sizeof("/.wineserver"); - if (!(server_dir = malloc( len ))) fatal_error( "out of memory\n" ); - strcpy( server_dir, config_dir ); - strcat( server_dir, "/.wineserver" ); + if (asprintf( &base_dir, "%s/.wineserver", config_dir ) == -1) + fatal_error( "out of memory\n" ); #else - len += sizeof("/tmp/.wine-") + 12; - if (!(server_dir = malloc( len ))) fatal_error( "out of memory\n" ); - sprintf( server_dir, "/tmp/.wine-%u", getuid() ); + if (asprintf( &base_dir, "/tmp/.wine-%u", getuid() ) == -1) + fatal_error( "out of memory\n" ); #endif - create_dir( server_dir, &st2 ); + create_dir( base_dir, &st2 ); /* now create the server directory */ - strcat( server_dir, "/server-" ); - p = server_dir + strlen(server_dir); - - sprintf( p, "%llx-%llx", (unsigned long long)st.st_dev, (unsigned long long)st.st_ino ); + if (asprintf( &server_dir, "%s/server-%llx-%llx", base_dir, + (unsigned long long)st.st_dev, (unsigned long long)st.st_ino ) == -1) + fatal_error( "out of memory\n" ); create_dir( server_dir, &st ); @@ -675,6 +670,7 @@ static char *create_server_dir( int force ) if (st.st_dev != st2.st_dev || st.st_ino != st2.st_ino) fatal_error( "chdir did not end up in %s\n", server_dir ); + free( base_dir ); free( config_dir ); return server_dir; } diff --git a/server/unicode.c b/server/unicode.c index f84520580d7..abc3367be75 100644 --- a/server/unicode.c +++ b/server/unicode.c @@ -265,11 +265,7 @@ static char *get_nls_dir(void) } *(++p) = 0; if (p > dir + 8 && !strcmp( p - 8, "/server/" )) nlsdir = "../nls"; /* inside build tree */ - if ((ret = malloc( strlen(dir) + strlen( nlsdir ) + 1 ))) - { - strcpy( ret, dir ); - strcat( ret, nlsdir ); - } + asprintf( &ret, "%s%s", dir, nlsdir ); free( dir ); return ret; } @@ -292,9 +288,7 @@ struct fd *load_intl_file(void) for (i = 0; i < ARRAY_SIZE( nls_dirs ); i++) { if (!nls_dirs[i]) continue; - if (!(path = malloc( strlen(nls_dirs[i]) + sizeof("/l_intl.nls" )))) continue; - strcpy( path, nls_dirs[i] ); - strcat( path, "/l_intl.nls" ); + if (asprintf( &path, "%s/l_intl.nls", nls_dirs[i] ) == -1) continue; if ((fd = open_fd( NULL, path, nt_name, O_RDONLY, &mode, FILE_READ_DATA, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ))) break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5356
From: Brendan Shanks <bshanks(a)codeweavers.com> --- server/change.c | 2 +- server/directory.c | 2 +- server/fd.c | 12 ++++++------ server/mapping.c | 2 +- server/process.c | 2 +- server/procfs.c | 4 ++-- server/ptrace.c | 4 ++-- server/registry.c | 17 +++++++++-------- server/trace.c | 2 +- server/unicode.c | 10 +++++----- 10 files changed, 29 insertions(+), 28 deletions(-) diff --git a/server/change.c b/server/change.c index 843e495411c..f773ccf8831 100644 --- a/server/change.c +++ b/server/change.c @@ -726,7 +726,7 @@ static char *get_path_from_fd( int fd, int sz ) #ifdef linux char *ret = malloc( 32 + sz ); - if (ret) sprintf( ret, "/proc/self/fd/%u", fd ); + if (ret) snprintf( ret, 32 + sz, "/proc/self/fd/%u", fd ); return ret; #elif defined(F_GETPATH) char *ret = malloc( PATH_MAX + sz ); diff --git a/server/directory.c b/server/directory.c index 23d7eb0a2b7..e521a7b38c9 100644 --- a/server/directory.c +++ b/server/directory.c @@ -320,7 +320,7 @@ static void create_session( unsigned int id ) release_object( dir_sessions ); } - sprintf( id_strA, "%u", id ); + snprintf( id_strA, sizeof(id_strA), "%u", id ); id_strW = ascii_to_unicode_str( id_strA, &id_str ); dir_id = create_directory( &dir_sessions->obj, &id_str, 0, HASH_SIZE, NULL ); dir_dosdevices = create_directory( &dir_id->obj, &dir_dosdevices_str, OBJ_PERMANENT, HASH_SIZE, NULL ); diff --git a/server/fd.c b/server/fd.c index 8576882aaa9..fc7f7be92b9 100644 --- a/server/fd.c +++ b/server/fd.c @@ -465,7 +465,7 @@ const char *get_timeout_str( timeout_t timeout ) { secs = -timeout / TICKS_PER_SEC; nsecs = -timeout % TICKS_PER_SEC; - sprintf( buffer, "+%ld.%07ld", secs, nsecs ); + snprintf( buffer, sizeof(buffer), "+%ld.%07ld", secs, nsecs ); } else /* absolute */ { @@ -477,12 +477,12 @@ const char *get_timeout_str( timeout_t timeout ) secs--; } if (secs >= 0) - sprintf( buffer, "%x%08x (+%ld.%07ld)", - (unsigned int)(timeout >> 32), (unsigned int)timeout, secs, nsecs ); + snprintf( buffer, sizeof(buffer), "%x%08x (+%ld.%07ld)", + (unsigned int)(timeout >> 32), (unsigned int)timeout, secs, nsecs ); else - sprintf( buffer, "%x%08x (-%ld.%07ld)", - (unsigned int)(timeout >> 32), (unsigned int)timeout, - -(secs + 1), TICKS_PER_SEC - nsecs ); + snprintf( buffer, sizeof(buffer), "%x%08x (-%ld.%07ld)", + (unsigned int)(timeout >> 32), (unsigned int)timeout, + -(secs + 1), TICKS_PER_SEC - nsecs ); } return buffer; } diff --git a/server/mapping.c b/server/mapping.c index 6b0de1b8b94..ff99b45ce51 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -292,7 +292,7 @@ static int make_temp_file( char name[16] ) value += (current_time >> 16) + current_time; for (i = 0; i < 0x8000 && fd < 0; i++, value += 7777) { - sprintf( name, "tmpmap-%08x", value ); + snprintf( name, 16, "tmpmap-%08x", value ); fd = open( name, O_RDWR | O_CREAT | O_EXCL, 0600 ); } return fd; diff --git a/server/process.c b/server/process.c index abd6edb58a9..b7c7df38e44 100644 --- a/server/process.c +++ b/server/process.c @@ -1552,7 +1552,7 @@ DECL_HANDLER(get_process_vm_counters) char proc_path[32], line[256]; unsigned long value; - sprintf( proc_path, "/proc/%u/status", process->unix_pid ); + snprintf( proc_path, sizeof(proc_path), "/proc/%u/status", process->unix_pid ); if ((f = fopen( proc_path, "r" ))) { while (fgets( line, sizeof(line), f )) diff --git a/server/procfs.c b/server/procfs.c index 7b61b2abb46..0492a69c94e 100644 --- a/server/procfs.c +++ b/server/procfs.c @@ -55,7 +55,7 @@ static int open_proc_as( struct process *process, int flags ) return -1; } - sprintf( buffer, "/proc/%u/as", process->unix_pid ); + snprintf( buffer, sizeof(buffer), "/proc/%u/as", process->unix_pid ); if ((fd = open( buffer, flags )) == -1) { if (errno == ENOENT) /* probably got killed */ @@ -75,7 +75,7 @@ static int open_proc_lwpctl( struct thread *thread ) if (thread->unix_pid == -1) return -1; - sprintf( buffer, "/proc/%u/lwp/%u/lwpctl", thread->unix_pid, thread->unix_tid ); + snprintf( buffer, sizeof(buffer), "/proc/%u/lwp/%u/lwpctl", thread->unix_pid, thread->unix_tid ); if ((fd = open( buffer, O_WRONLY )) == -1) { if (errno == ENOENT) /* probably got killed */ diff --git a/server/ptrace.c b/server/ptrace.c index 976d3a3778e..bd86f0bf2ec 100644 --- a/server/ptrace.c +++ b/server/ptrace.c @@ -366,7 +366,7 @@ int read_process_memory( struct process *process, client_ptr_t ptr, data_size_t char procmem[24]; int fd; - sprintf( procmem, "/proc/%u/mem", process->unix_pid ); + snprintf( procmem, sizeof(procmem), "/proc/%u/mem", process->unix_pid ); if ((fd = open( procmem, O_RDONLY )) != -1) { ssize_t ret = pread( fd, dest, size, ptr ); @@ -467,7 +467,7 @@ int write_process_memory( struct process *process, client_ptr_t ptr, data_size_t char procmem[24]; int fd; - sprintf( procmem, "/proc/%u/mem", process->unix_pid ); + snprintf( procmem, sizeof(procmem), "/proc/%u/mem", process->unix_pid ); if ((fd = open( procmem, O_WRONLY )) != -1) { ssize_t r = pwrite( fd, src, size, ptr ); diff --git a/server/registry.c b/server/registry.c index e88bed1e72e..5e2c748fa32 100644 --- a/server/registry.c +++ b/server/registry.c @@ -1831,15 +1831,16 @@ static int load_init_registry_from_file( const char *filename, struct key *key ) static WCHAR *format_user_registry_path( const struct sid *sid, struct unicode_str *path ) { - char buffer[7 + 11 + 11 + 11 * ARRAY_SIZE(sid->sub_auth)], *p = buffer; + char buffer[7 + 11 + 11 + 11 * ARRAY_SIZE(sid->sub_auth)]; unsigned int i; - - p += sprintf( p, "User\\S-%u-%u", sid->revision, - ((unsigned int)sid->id_auth[2] << 24) | - ((unsigned int)sid->id_auth[3] << 16) | - ((unsigned int)sid->id_auth[4] << 8) | - ((unsigned int)sid->id_auth[5]) ); - for (i = 0; i < sid->sub_count; i++) p += sprintf( p, "-%u", sid->sub_auth[i] ); + int len; + + len = snprintf( buffer, sizeof(buffer), "User\\S-%u-%u", sid->revision, + ((unsigned int)sid->id_auth[2] << 24) | + ((unsigned int)sid->id_auth[3] << 16) | + ((unsigned int)sid->id_auth[4] << 8) | + ((unsigned int)sid->id_auth[5]) ); + for (i = 0; i < sid->sub_count; i++) len += snprintf( buffer + len, sizeof(buffer) - len, "-%u", sid->sub_auth[i] ); return ascii_to_unicode_str( buffer, path ); } diff --git a/server/trace.c b/server/trace.c index a94b535bd5d..efb4bb03c4a 100644 --- a/server/trace.c +++ b/server/trace.c @@ -5645,7 +5645,7 @@ static const char *get_status_name( unsigned int status ) for (i = 0; status_names[i].name; i++) if (status_names[i].value == status) return status_names[i].name; } - sprintf( buffer, "%x", status ); + snprintf( buffer, sizeof(buffer), "%x", status ); return buffer; } diff --git a/server/unicode.c b/server/unicode.c index abc3367be75..cae9df7da65 100644 --- a/server/unicode.c +++ b/server/unicode.c @@ -212,19 +212,19 @@ int dump_strW( const WCHAR *str, data_size_t len, FILE *f, const char escape[2] if (*str > 127) /* hex escape */ { if (len > 1 && str[1] < 128 && isxdigit((char)str[1])) - pos += sprintf( pos, "\\x%04x", *str ); + pos += snprintf( pos, sizeof(buffer) - (pos - buffer), "\\x%04x", *str ); else - pos += sprintf( pos, "\\x%x", *str ); + pos += snprintf( pos, sizeof(buffer) - (pos - buffer), "\\x%x", *str ); continue; } if (*str < 32) /* octal or C escape */ { if (escapes[*str] != '.') - pos += sprintf( pos, "\\%c", escapes[*str] ); + pos += snprintf( pos, sizeof(buffer) - (pos - buffer), "\\%c", escapes[*str] ); else if (len > 1 && str[1] >= '0' && str[1] <= '7') - pos += sprintf( pos, "\\%03o", *str ); + pos += snprintf( pos, sizeof(buffer) - (pos - buffer), "\\%03o", *str ); else - pos += sprintf( pos, "\\%o", *str ); + pos += snprintf( pos, sizeof(buffer) - (pos - buffer), "\\%o", *str ); continue; } if (*str == '\\' || *str == escape[0] || *str == escape[1]) *pos++ = '\\'; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5356
From: Brendan Shanks <bshanks(a)codeweavers.com> --- server/registry.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/server/registry.c b/server/registry.c index 5e2c748fa32..d9d7b4822d1 100644 --- a/server/registry.c +++ b/server/registry.c @@ -137,7 +137,7 @@ static struct key_value *find_value( const struct key *key, const struct unicode struct save_branch_info { struct key *key; - const char *path; + const char *filename; }; #define MAX_SAVE_BRANCH_INFO 3 @@ -1823,7 +1823,7 @@ static int load_init_registry_from_file( const char *filename, struct key *key ) assert( save_branch_count < MAX_SAVE_BRANCH_INFO ); - save_branch_info[save_branch_count].path = filename; + save_branch_info[save_branch_count].filename = filename; save_branch_info[save_branch_count++].key = (struct key *)grab_object( key ); make_object_permanent( &key->obj ); return (f != NULL); @@ -2038,10 +2038,10 @@ static void save_registry( struct key *key, obj_handle_t handle ) } /* save a registry branch to a file */ -static int save_branch( struct key *key, const char *path ) +static int save_branch( struct key *key, const char *filename ) { struct stat st; - char *p, *tmp = NULL; + char tmp[20] = ""; int fd, count = 0, ret = 0; FILE *f; @@ -2053,11 +2053,11 @@ static int save_branch( struct key *key, const char *path ) /* test the file type */ - if ((fd = open( path, O_WRONLY )) != -1) + if ((fd = open( filename, O_WRONLY )) != -1) { /* if file is not a regular file or has multiple links or is accessed * via symbolic links, write directly into it; otherwise use a temp file */ - if (!lstat( path, &st ) && (!S_ISREG(st.st_mode) || st.st_nlink > 1)) + if (!lstat( filename, &st ) && (!S_ISREG(st.st_mode) || st.st_nlink > 1)) { ftruncate( fd, 0 ); goto save; @@ -2065,15 +2065,11 @@ static int save_branch( struct key *key, const char *path ) close( fd ); } - /* create a temp file in the same directory */ + /* create a temp file */ - if (!(tmp = malloc( strlen(path) + 20 ))) goto done; - strcpy( tmp, path ); - if ((p = strrchr( tmp, '/' ))) p++; - else p = tmp; for (;;) { - sprintf( p, "reg%lx%04x.tmp", (long) getpid(), count++ ); + snprintf( tmp, sizeof(tmp), "reg%lx%04x.tmp", (long) getpid(), count++ ); if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0666 )) != -1) break; if (errno != EEXIST) goto done; close( fd ); @@ -2084,29 +2080,28 @@ static int save_branch( struct key *key, const char *path ) save: if (!(f = fdopen( fd, "w" ))) { - if (tmp) unlink( tmp ); + if (tmp[0]) unlink( tmp ); close( fd ); goto done; } if (debug_level > 1) { - fprintf( stderr, "%s: ", path ); + fprintf( stderr, "%s: ", filename ); dump_operation( key, NULL, "saving" ); } save_all_subkeys( key, f ); ret = !fclose(f); - if (tmp) + if (tmp[0]) { /* if successfully written, rename to final name */ - if (ret) ret = !rename( tmp, path ); + if (ret) ret = !rename( tmp, filename ); if (!ret) unlink( tmp ); } done: - free( tmp ); if (ret) make_clean( key ); return ret; } @@ -2119,7 +2114,7 @@ static void periodic_save( void *arg ) if (fchdir( config_dir_fd ) == -1) return; save_timeout_user = NULL; for (i = 0; i < save_branch_count; i++) - save_branch( save_branch_info[i].key, save_branch_info[i].path ); + save_branch( save_branch_info[i].key, save_branch_info[i].filename ); if (fchdir( server_dir_fd ) == -1) fatal_error( "chdir to server dir: %s\n", strerror( errno )); set_periodic_save_timer(); } @@ -2139,10 +2134,10 @@ void flush_registry(void) if (fchdir( config_dir_fd ) == -1) return; for (i = 0; i < save_branch_count; i++) { - if (!save_branch( save_branch_info[i].key, save_branch_info[i].path )) + if (!save_branch( save_branch_info[i].key, save_branch_info[i].filename )) { fprintf( stderr, "wineserver: could not save registry branch to %s", - save_branch_info[i].path ); + save_branch_info[i].filename ); perror( " " ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5356
On Thu Mar 28 05:28:05 2024 +0000, Alexandre Julliard wrote:
There has to be a nicer way to do this. I modified this to do separate `asprintf` calls for the base dir and server dir.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5356#note_66344
participants (2)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)