From 9d01d090c48c74a29b4ef67e0cd204772a2193c3 Mon Sep 17 00:00:00 2001
From: Simon Howard
Date: Fri, 24 Oct 2014 20:29:56 -0400
Subject: Replace strdup() with M_StringDuplicate().

strdup() can theoretically fail and return NULL. This could lead to
a crash or undesirable behavior. Add M_StringDuplicate() which does
the same thing but exits with an error if a string cannot be
allocated.

This fixes #456. Thanks to Quasar for the suggestion.
---
 HACKING                 |  1 +
 src/d_iwad.c            |  8 ++++----
 src/deh_io.c            |  2 +-
 src/doom/d_main.c       |  2 +-
 src/gusconf.c           |  3 ++-
 src/heretic/g_game.c    |  2 +-
 src/i_sdlmusic.c        | 12 ++++++------
 src/m_config.c          |  6 +++---
 src/m_misc.c            | 20 ++++++++++++++++++++
 src/m_misc.h            |  1 +
 src/net_query.c         |  3 ++-
 src/net_sdl.c           |  2 +-
 src/net_server.c        |  2 +-
 src/setup/execute.c     |  2 +-
 src/setup/multiplayer.c | 10 +++++-----
 src/setup/sound.c       |  5 +++--
 16 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/HACKING b/HACKING
index a7284ba5..3bbe23d0 100644
--- a/HACKING
+++ b/HACKING
@@ -130,6 +130,7 @@ avoided when writing code for Chocolate Doom. These are:
     strncpy()             M_StringCopy()
     strcat()              M_StringConcat()
     strncat()             M_StringConcat()
+    strdup()              M_StringDuplicate()
 
 Lots of the code includes calls to DEH_String() to simulate string
 replacement by the Dehacked tool. Be careful when using Dehacked
diff --git a/src/d_iwad.c b/src/d_iwad.c
index b81f6f6f..9cdffcb8 100644
--- a/src/d_iwad.c
+++ b/src/d_iwad.c
@@ -413,7 +413,7 @@ static char *CheckDirectoryHasIWAD(char *dir, char *iwadname)
 
     if (DirIsFile(dir, iwadname) && M_FileExists(dir))
     {
-        return strdup(dir);
+        return M_StringDuplicate(dir);
     }
 
     // Construct the full path to the IWAD if it is located in
@@ -421,7 +421,7 @@ static char *CheckDirectoryHasIWAD(char *dir, char *iwadname)
 
     if (!strcmp(dir, "."))
     {
-        filename = strdup(iwadname);
+        filename = M_StringDuplicate(iwadname);
     }
     else
     {
@@ -523,7 +523,7 @@ static void AddDoomWadPath(void)
         return;
     }
 
-    doomwadpath = strdup(doomwadpath);
+    doomwadpath = M_StringDuplicate(doomwadpath);
 
     // Add the initial directory
 
@@ -640,7 +640,7 @@ char *D_FindWADByName(char *name)
 
         if (DirIsFile(iwad_dirs[i], name) && M_FileExists(iwad_dirs[i]))
         {
-            return strdup(iwad_dirs[i]);
+            return M_StringDuplicate(iwad_dirs[i]);
         }
 
         // Construct a string for the full path
diff --git a/src/deh_io.c b/src/deh_io.c
index 526d8e46..c5e81b0a 100644
--- a/src/deh_io.c
+++ b/src/deh_io.c
@@ -97,7 +97,7 @@ deh_context_t *DEH_OpenFile(char *filename)
 
     context->type = DEH_INPUT_FILE;
     context->stream = fstream;
-    context->filename = strdup(filename);
+    context->filename = M_StringDuplicate(filename);
 
     return context;
 }
diff --git a/src/doom/d_main.c b/src/doom/d_main.c
index 10606161..1e9e950e 100644
--- a/src/doom/d_main.c
+++ b/src/doom/d_main.c
@@ -1123,7 +1123,7 @@ static void LoadIwadDeh(void)
         }
         else
         {
-            chex_deh = strdup("chex.deh");
+            chex_deh = M_StringDuplicate("chex.deh");
         }
 
         // If the dehacked patch isn't found, try searching the WAD
diff --git a/src/gusconf.c b/src/gusconf.c
index 70cdd87c..a2dfdac3 100644
--- a/src/gusconf.c
+++ b/src/gusconf.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <ctype.h>
 
+#include "m_misc.h"
 #include "w_wad.h"
 #include "z_zone.h"
 
@@ -122,7 +123,7 @@ static void ParseLine(gus_config_t *config, char *line)
     mapped_id = atoi(fields[MappingIndex()]);
 
     free(config->patch_names[instr_id]);
-    config->patch_names[instr_id] = strdup(fields[5]);
+    config->patch_names[instr_id] = M_StringDuplicate(fields[5]);
     config->mapping[instr_id] = mapped_id;
 }
 
diff --git a/src/heretic/g_game.c b/src/heretic/g_game.c
index da96537a..389c0e1d 100644
--- a/src/heretic/g_game.c
+++ b/src/heretic/g_game.c
@@ -1429,7 +1429,7 @@ static char *savename = NULL;
 
 void G_LoadGame(char *name)
 {
-    savename = strdup(name);
+    savename = M_StringDuplicate(name);
     gameaction = ga_loadgame;
 }
 
diff --git a/src/i_sdlmusic.c b/src/i_sdlmusic.c
index b8c2b2f7..e70b2c05 100644
--- a/src/i_sdlmusic.c
+++ b/src/i_sdlmusic.c
@@ -498,14 +498,14 @@ static char *GetFullPath(char *base_filename, char *path)
     // so just return it.
     if (path[0] == DIR_SEPARATOR)
     {
-        return strdup(path);
+        return M_StringDuplicate(path);
     }
 
 #ifdef _WIN32
     // d:\path\...
     if (isalpha(path[0]) && path[1] == ':' && path[2] == DIR_SEPARATOR)
     {
-        return strdup(path);
+        return M_StringDuplicate(path);
     }
 #endif
 
@@ -516,7 +516,7 @@ static char *GetFullPath(char *base_filename, char *path)
 
     // Copy config filename and cut off the filename to just get the
     // parent dir.
-    basedir = strdup(base_filename);
+    basedir = M_StringDuplicate(base_filename);
     p = strrchr(basedir, DIR_SEPARATOR);
     if (p != NULL)
     {
@@ -525,7 +525,7 @@ static char *GetFullPath(char *base_filename, char *path)
     }
     else
     {
-        result = strdup(path);
+        result = M_StringDuplicate(path);
     }
     free(basedir);
     free(path);
@@ -672,7 +672,7 @@ static void LoadSubstituteConfigs(void)
 
     if (!strcmp(configdir, ""))
     {
-        musicdir = strdup("");
+        musicdir = M_StringDuplicate("");
     }
     else
     {
@@ -805,7 +805,7 @@ static boolean WriteWrapperTimidityConfig(char *write_path)
     p = strrchr(timidity_cfg_path, DIR_SEPARATOR);
     if (p != NULL)
     {
-        path = strdup(timidity_cfg_path);
+        path = M_StringDuplicate(timidity_cfg_path);
         path[p - timidity_cfg_path] = '\0';
         fprintf(fstream, "dir %s\n", path);
         free(path);
diff --git a/src/m_config.c b/src/m_config.c
index a2188f5b..083d298a 100644
--- a/src/m_config.c
+++ b/src/m_config.c
@@ -1732,7 +1732,7 @@ static void SetVariable(default_t *def, char *value)
     switch (def->type)
     {
         case DEFAULT_STRING:
-            * (char **) def->location = strdup(value);
+            * (char **) def->location = M_StringDuplicate(value);
             break;
 
         case DEFAULT_INT:
@@ -2062,7 +2062,7 @@ static char *GetDefaultConfigDir(void)
     else
 #endif /* #ifndef _WIN32 */
     {
-        return strdup("");
+        return M_StringDuplicate("");
     }
 }
 
@@ -2111,7 +2111,7 @@ char *M_GetSaveGameDir(char *iwadname)
 
     if (!strcmp(configdir, ""))
     {
-	savegamedir = strdup("");
+	savegamedir = M_StringDuplicate("");
     }
     else
     {
diff --git a/src/m_misc.c b/src/m_misc.c
index 8fa526dc..2e363412 100644
--- a/src/m_misc.c
+++ b/src/m_misc.c
@@ -284,6 +284,26 @@ char *M_StrCaseStr(char *haystack, char *needle)
     return NULL;
 }
 
+//
+// Safe version of strdup() that checks the string was successfully
+// allocated.
+//
+
+char *M_StringDuplicate(const char *orig)
+{
+    char *result;
+
+    result = strdup(orig);
+
+    if (result == NULL)
+    {
+        I_Error("Failed to duplicate string (length %i)\n",
+                strlen(orig));
+    }
+
+    return result;
+}
+
 //
 // String replace function.
 //
diff --git a/src/m_misc.h b/src/m_misc.h
index 99151f56..844b4859 100644
--- a/src/m_misc.h
+++ b/src/m_misc.h
@@ -35,6 +35,7 @@ boolean M_StrToInt(const char *str, int *result);
 void M_ExtractFileBase(char *path, char *dest);
 void M_ForceUppercase(char *text);
 char *M_StrCaseStr(char *haystack, char *needle);
+char *M_StringDuplicate(const char *orig);
 boolean M_StringCopy(char *dest, const char *src, size_t dest_size);
 boolean M_StringConcat(char *dest, const char *src, size_t dest_size);
 char *M_StringReplace(const char *haystack, const char *needle,
diff --git a/src/net_query.c b/src/net_query.c
index f44d3b98..d5288e25 100644
--- a/src/net_query.c
+++ b/src/net_query.c
@@ -22,6 +22,7 @@
 
 #include "i_system.h"
 #include "i_timer.h"
+#include "m_misc.h"
 
 #include "net_common.h"
 #include "net_defs.h"
@@ -900,7 +901,7 @@ boolean NET_StartSecureDemo(prng_seed_t seed)
 
             if (signature != NULL)
             {
-                securedemo_start_message = strdup(signature);
+                securedemo_start_message = M_StringDuplicate(signature);
                 result = true;
             }
         }
diff --git a/src/net_sdl.c b/src/net_sdl.c
index 3c381e69..7de61cb7 100644
--- a/src/net_sdl.c
+++ b/src/net_sdl.c
@@ -325,7 +325,7 @@ net_addr_t *NET_SDL_ResolveAddress(char *address)
 
     if (colon != NULL)
     {
-	addr_hostname = strdup(address);
+	addr_hostname = M_StringDuplicate(address);
 	addr_hostname[colon - address] = '\0';
 	addr_port = atoi(colon + 1);
     }
diff --git a/src/net_server.c b/src/net_server.c
index ae8b5897..b4496bb5 100644
--- a/src/net_server.c
+++ b/src/net_server.c
@@ -563,7 +563,7 @@ static void NET_SV_InitNewClient(net_client_t *client,
     NET_Conn_InitServer(&client->connection, addr);
     client->addr = addr;
     client->last_send_time = -1;
-    client->name = strdup(player_name);
+    client->name = M_StringDuplicate(player_name);
 
     // init the ticcmd send queue
 
diff --git a/src/setup/execute.c b/src/setup/execute.c
index 1fef6e44..bbb23c38 100644
--- a/src/setup/execute.c
+++ b/src/setup/execute.c
@@ -268,7 +268,7 @@ static char *GetFullExePath(const char *program)
 
     if (sep == NULL)
     {
-        result = strdup(program);
+        result = M_StringDuplicate(program);
     }
     else
     {
diff --git a/src/setup/multiplayer.c b/src/setup/multiplayer.c
index d833a495..f116ffb2 100644
--- a/src/setup/multiplayer.c
+++ b/src/setup/multiplayer.c
@@ -856,7 +856,7 @@ static void SelectQueryAddress(TXT_UNCAST_ARG(button),
     // Set address to connect to:
 
     free(connect_address);
-    connect_address = strdup(button->label);
+    connect_address = M_StringDuplicate(button->label);
 
     // Auto-choose IWAD if there is already a player connected.
 
@@ -1044,7 +1044,7 @@ void SetChatMacroDefaults(void)
     {
         if (chat_macros[i] == NULL)
         {
-            chat_macros[i] = strdup(defaults[i]);
+            chat_macros[i] = M_StringDuplicate(defaults[i]);
         }
     }
 }
@@ -1053,12 +1053,12 @@ void SetPlayerNameDefault(void)
 {
     if (net_player_name == NULL)
     {
-        net_player_name = strdup(getenv("USER"));
+        net_player_name = M_StringDuplicate(getenv("USER"));
     }
 
     if (net_player_name == NULL)
     {
-        net_player_name = strdup(getenv("USERNAME"));
+        net_player_name = M_StringDuplicate(getenv("USERNAME"));
     }
 
     // On Windows, environment variables are in OEM codepage
@@ -1073,7 +1073,7 @@ void SetPlayerNameDefault(void)
 
     if (net_player_name == NULL)
     {
-        net_player_name = strdup("player");
+        net_player_name = M_StringDuplicate("player");
     }
 }
 
diff --git a/src/setup/sound.c b/src/setup/sound.c
index 76fc0564..2b9bfcbb 100644
--- a/src/setup/sound.c
+++ b/src/setup/sound.c
@@ -20,6 +20,7 @@
 
 #include "textscreen.h"
 #include "m_config.h"
+#include "m_misc.h"
 
 #include "mode.h"
 #include "sound.h"
@@ -324,8 +325,8 @@ void BindSoundVariables(void)
         M_BindVariable("show_talk",       &show_talk);
     }
 
-    timidity_cfg_path = strdup("");
-    gus_patch_path = strdup("");
+    timidity_cfg_path = M_StringDuplicate("");
+    gus_patch_path = M_StringDuplicate("");
 
     // Default sound volumes - different games use different values.
 
-- 
cgit v1.2.3