[PATCH 0/1] MR4261: ntdll/unix: Replace some malloc/sprintf calls with asprintf.

[Brendan Shanks (＠bshanks)]

When changing some sprintf calls to snprintf, I realized there are many cases that would be simplified by using asprintf instead. It's been present in glibc since the 90s, the BSDs/macOS for almost as long, and Solaris since 'Solaris 10 8/2011'.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4261

[PATCH 1/1] ntdll/unix: Replace some malloc/sprintf calls with asprintf.

[Brendan Shanks]

From: Brendan Shanks <bshanks(a)codeweavers.com>

---
 dlls/ntdll/unix/env.c    | 10 +++-------
 dlls/ntdll/unix/file.c   | 14 ++++----------
 dlls/ntdll/unix/loader.c | 21 +++++++++------------
 dlls/ntdll/unix/server.c | 30 +++++++++++++-----------------
 4 files changed, 29 insertions(+), 46 deletions(-)

diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c
index f845d2372e1..ec908a5389f 100644
--- a/dlls/ntdll/unix/env.c
+++ b/dlls/ntdll/unix/env.c
@@ -108,8 +108,7 @@ static char *get_nls_file_path( UINT type, UINT id )
         break;
     }
     if (!name) return NULL;
-    if (!(path = malloc( strlen(dir) + strlen(name) + 10 ))) return NULL;
-    sprintf( path, "%s/nls/%s.nls", dir, name );
+    if (asprintf( &path, "%s/nls/%s.nls", dir, name ) == -1) return NULL;
     return path;
 }
 
@@ -121,8 +120,7 @@ static void *read_nls_file( const char *name )
     void *data, *ret = NULL;
     int fd;
 
-    if (!(path = malloc( strlen(dir) + strlen(name) + 10 ))) return NULL;
-    sprintf( path, "%s/nls/%s", dir, name );
+    if (asprintf( &path, "%s/nls/%s", dir, name ) == -1) return NULL;
 
     if ((fd = open( path, O_RDONLY )) != -1)
     {
@@ -2238,9 +2236,7 @@ NTSTATUS WINAPI NtInitializeNlsFiles( void **ptr, LCID *lcid, LARGE_INTEGER *siz
     SIZE_T mapsize;
     NTSTATUS status;
 
-    if (!(path = malloc( strlen(dir) + sizeof("/nls/locale.nls") ))) return STATUS_NO_MEMORY;
-    strcpy( path, dir );
-    strcat( path, "/nls/locale.nls" );
+    if (asprintf( &path, "%s/nls/locale.nls", dir ) == -1) return STATUS_NO_MEMORY;
     status = open_nls_data_file( path, system_dir, &file );
     free( path );
     if (!status)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index c6533710365..72fcba6e998 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -2098,10 +2098,8 @@ static unsigned int get_drives_info( struct file_identity info[MAX_DOS_DRIVES] )
         struct stat st;
         unsigned int i;
 
-        if ((buffer = malloc( strlen(config_dir) + sizeof("/dosdevices/a:") )))
+        if (asprintf( &buffer, "%s/dosdevices/a:", config_dir ) != -1)
         {
-            strcpy( buffer, config_dir );
-            strcat( buffer, "/dosdevices/a:" );
             p = buffer + strlen(buffer) - 2;
 
             for (i = nb_drives = 0; i < MAX_DOS_DRIVES; i++)
@@ -2898,9 +2896,7 @@ static void init_redirects(void)
     char *dir;
     struct stat st;
 
-    if (!(dir = malloc( strlen(config_dir) + sizeof(system_dir) ))) return;
-    strcpy( dir, config_dir );
-    strcat( dir, system_dir );
+    if (asprintf( &dir, "%s%s", config_dir, system_dir ) == -1) return;
     if (!stat( dir, &st ))
     {
         sysdir.dev = st.st_dev;
@@ -3904,11 +3900,9 @@ static NTSTATUS unmount_device( HANDLE handle )
 #else
                 static const char umount[] = "umount >/dev/null 2>&1 ";
 #endif
-                char *cmd = malloc( strlen(mount_point)+sizeof(umount));
-                if (cmd)
+                char *cmd;
+                if (asprintf( &cmd, "%s%s", umount, mount_point ) != -1)
                 {
-                    strcpy( cmd, umount );
-                    strcat( cmd, mount_point );
                     system( cmd );
                     free( cmd );
 #ifdef linux
diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c
index bfa161e25c9..0db25f88527 100644
--- a/dlls/ntdll/unix/loader.c
+++ b/dlls/ntdll/unix/loader.c
@@ -663,9 +663,7 @@ static void init_paths( char *argv[] )
     }
     else wineloader = build_path( build_path( build_dir, "loader" ), basename );
 
-    env = malloc( sizeof("WINELOADER=") + strlen(wineloader) );
-    strcpy( env, "WINELOADER=" );
-    strcat( env, wineloader );
+    asprintf( &env, "WINELOADER=%s", wineloader );
     putenv( env );
 
     set_dll_path();
@@ -1835,18 +1833,18 @@ static void load_ntdll(void)
     UNICODE_STRING str;
     void *module;
     SIZE_T size = 0;
-    char *name;
+    char *name = NULL;
 
     init_unicode_string( &str, path );
     InitializeObjectAttributes( &attr, &str, 0, 0, NULL );
 
-    name = malloc( strlen( ntdll_dir ) + strlen( pe_dir ) + sizeof("/ntdll.dll.so") );
-    if (build_dir) sprintf( name, "%s%s/ntdll.dll", ntdll_dir, pe_dir );
-    else sprintf( name, "%s%s/ntdll.dll", dll_dir, pe_dir );
+    if (build_dir) asprintf( &name, "%s%s/ntdll.dll", ntdll_dir, pe_dir );
+    else asprintf( &name, "%s%s/ntdll.dll", dll_dir, pe_dir );
     status = open_builtin_pe_file( name, &attr, &module, &size, &info, 0, 0, current_machine, FALSE );
     if (status == STATUS_DLL_NOT_FOUND)
     {
-        sprintf( name, "%s/ntdll.dll.so", ntdll_dir );
+        free( name );
+        asprintf( &name, "%s/ntdll.dll.so", ntdll_dir );
         status = open_builtin_so_file( name, &attr, &module, &info, FALSE );
     }
     if (status == STATUS_IMAGE_NOT_AT_BASE) status = virtual_relocate_module( module );
@@ -1873,16 +1871,15 @@ static void load_apiset_dll(void)
     unsigned int status;
     HANDLE handle, mapping;
     SIZE_T size;
-    char *name;
+    char *name = NULL;
     void *ptr;
     UINT i;
 
     init_unicode_string( &str, path );
     InitializeObjectAttributes( &attr, &str, 0, 0, NULL );
 
-    name = malloc( strlen( ntdll_dir ) + strlen( pe_dir ) + sizeof("/apisetschema/apisetschema.dll") );
-    if (build_dir) sprintf( name, "%s/dlls/apisetschema%s/apisetschema.dll", build_dir, pe_dir );
-    else sprintf( name, "%s%s/apisetschema.dll", dll_dir, pe_dir );
+    if (build_dir) asprintf( &name, "%s/dlls/apisetschema%s/apisetschema.dll", build_dir, pe_dir );
+    else asprintf( &name, "%s%s/apisetschema.dll", dll_dir, pe_dir );
     status = open_unix_file( &handle, name, GENERIC_READ | SYNCHRONIZE, &attr, 0,
                              FILE_SHARE_READ | FILE_SHARE_DELETE, FILE_OPEN,
                              FILE_SYNCHRONOUS_IO_NONALERT | FILE_NON_DIRECTORY_FILE, NULL, 0 );
diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c
index 24653904308..7211457387c 100644
--- a/dlls/ntdll/unix/server.c
+++ b/dlls/ntdll/unix/server.c
@@ -1243,29 +1243,25 @@ int server_pipe( int fd[2] )
  */
 static const char *init_server_dir( dev_t dev, ino_t ino )
 {
-    char *p, *dir;
-    size_t len = sizeof("/server-") + 2 * sizeof(dev) + 2 * sizeof(ino) + 2;
+    char *dir = NULL;
+    int p;
+    char tmp[2 * sizeof(dev) + 2 * sizeof(ino) + 2];
 
-#ifdef __ANDROID__  /* there's no /tmp dir on Android */
-    len += strlen( config_dir ) + sizeof("/.wineserver");
-    dir = malloc( len );
-    strcpy( dir, config_dir );
-    strcat( dir, "/.wineserver/server-" );
-#else
-    len += sizeof("/tmp/.wine-") + 12;
-    dir = malloc( len );
-    sprintf( dir, "/tmp/.wine-%u/server-", getuid() );
-#endif
-    p = dir + strlen( dir );
     if (dev != (unsigned long)dev)
-        p += sprintf( p, "%lx%08lx-", (unsigned long)((unsigned long long)dev >> 32), (unsigned long)dev );
+        p = snprintf( tmp, sizeof(tmp), "%lx%08lx-", (unsigned long)((unsigned long long)dev >> 32), (unsigned long)dev );
     else
-        p += sprintf( p, "%lx-", (unsigned long)dev );
+        p = snprintf( tmp, sizeof(tmp), "%lx-", (unsigned long)dev );
 
     if (ino != (unsigned long)ino)
-        sprintf( p, "%lx%08lx", (unsigned long)((unsigned long long)ino >> 32), (unsigned long)ino );
+        snprintf( tmp + p, sizeof(tmp) - p, "%lx%08lx", (unsigned long)((unsigned long long)ino >> 32), (unsigned long)ino );
     else
-        sprintf( p, "%lx", (unsigned long)ino );
+        snprintf( tmp + p, sizeof(tmp) - p, "%lx", (unsigned long)ino );
+
+#ifdef __ANDROID__  /* there's no /tmp dir on Android */
+    asprintf( &dir, "%s/.wineserver/server-%s", config_dir, tmp );
+#else
+    asprintf( &dir, "/tmp/.wine-%u/server-%s", getuid(), tmp );
+#endif
     return dir;
 }
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4261