common: make COM_StripExtension work in-place

Rather than needing external checks on the output buffer, or adding a
"max length" parameter and needing to check for overflow, etc., make
COM_StripExtension modify the existing string.

Callers updated as needed.

A personal note that I finally realised that snprintf always zero
terminates the output buffer! So that should tidy up existing code a
little more and makes it an even better replacement for strncpy in
almost every case.

Signed-off-by: Kevin Shanahan <kmshanah@disenchant.net>

NQ/cl_parse.c

@@ -236,6 +236,7 @@ void
 CL_ParseServerInfo(void)
 {
     char *level;
+    const char *mapname;
     int i, len, maxlen;
     int nummodels, numsounds;
     char model_precache[MAX_MODELS][MAX_QPATH];
@@ -325,7 +326,9 @@ CL_ParseServerInfo(void)
     }
 
 // copy the naked name of the map file to the cl structure
-    COM_StripExtension(COM_SkipPath(model_precache[1]), cl.mapname);
+    mapname = COM_SkipPath(model_precache[1]);
+    snprintf(cl.mapname, sizeof(cl.mapname), "%s", mapname);
+    COM_StripExtension(cl.mapname);
 
 //
 // now we try to load everything else until a cache allocation fails

QW/client/cl_parse.c

@@ -171,6 +171,7 @@ qboolean
 CL_CheckOrDownloadFile(char *filename)
 {
     FILE *f;
+    int maxlen;
 
     if (strstr(filename, "..")) {
 	Con_Printf("Refusing to download a path with ..\n");
@@ -182,23 +183,32 @@ CL_CheckOrDownloadFile(char *filename)
 	fclose(f);
 	return true;
     }
-    //ZOID - can't download when recording
+    /* can't download when recording */
     if (cls.demorecording) {
 	Con_Printf("Unable to download %s in record mode.\n",
 		   cls.downloadname);
 	return true;
     }
-    //ZOID - can't download when playback
+    /* can't download when playback */
     if (cls.demoplayback)
 	return true;
 
-    strcpy(cls.downloadname, filename);
+    snprintf(cls.downloadname, sizeof(cls.downloadname), "%s", filename);
     Con_Printf("Downloading %s...\n", cls.downloadname);
 
-    // download to a temp name, and only rename
-    // to the real name when done, so if interrupted
-    // a runt file wont be left
-    COM_StripExtension(cls.downloadname, cls.downloadtempname);
+    /*
+     * download to a temp name, and only rename to the real name when
+     * done, so if interrupted a runt file wont be left
+     */
+    strcpy(cls.downloadtempname, cls.downloadname); /* same size */
+    COM_StripExtension(cls.downloadtempname);
+
+    /* make sure the .tmp extension fits... */
+    maxlen = sizeof(cls.downloadtempname);
+    if (strlen(cls.downloadtempname) + strlen(".tmp") > maxlen - 1) {
+	Con_Printf("Refusing download, pathname too long\n");
+	return true;
+    }
     strcat(cls.downloadtempname, ".tmp");
 
     MSG_WriteByte(&cls.netchan.message, clc_stringcmd);
@@ -892,21 +902,21 @@ CL_NewTranslation(int slot)
 
     R_TranslatePlayerSkin(slot);
 #else
-
     int i, j;
     int top, bottom;
     byte *dest, *source;
     player_info_t *player;
-    char s[512];
+    char *skin;
 
     if (slot > MAX_CLIENTS)
 	Sys_Error("%s: slot > MAX_CLIENTS", __func__);
 
     player = &cl.players[slot];
 
-    strcpy(s, Info_ValueForKey(player->userinfo, "skin"));
-    COM_StripExtension(s, s);
-    if (player->skin && !strcasecmp(s, player->skin->name))
+    skin = Info_ValueForKey(player->userinfo, "skin");
+    COM_StripExtension(skin);
+
+    if (player->skin && !strcasecmp(skin, player->skin->name))
 	player->skin = NULL;
 
     if (player->_topcolor != player->topcolor ||
@@ -927,7 +937,8 @@ CL_NewTranslation(int slot)
 	bottom *= 16;
 
 	for (i = 0; i < VID_GRADES; i++, dest += 256, source += 256) {
-	    if (top < 128)	// the artists made some backwards ranges.  sigh.
+	    /* the artists made some backwards ranges.  sigh. */
+	    if (top < 128)
 		memcpy(dest + TOP_RANGE, source + top, 16);
 	    else
 		for (j = 0; j < 16; j++)

QW/client/skin.c

@@ -28,7 +28,7 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 cvar_t baseskin = { "baseskin", "base" };
 cvar_t noskins = { "noskins", "0" };
 
-char allskins[128];
+static char allskins[128];
 
 #define	MAX_CACHED_SKINS		128
 skin_t skins[MAX_CACHED_SKINS];
@@ -48,22 +48,20 @@ Skin_Find(player_info_t * sc)
 {
     skin_t *skin;
     int i;
-    char name[128], *s;
+    char name[128];
+    const char *skinname;
 
-    if (allskins[0])
-	strcpy(name, allskins);
-    else {
-	s = Info_ValueForKey(sc->userinfo, "skin");
-	if (s && s[0])
-	    strcpy(name, s);
-	else
-	    strcpy(name, baseskin.string);
+    skinname = allskins;
+    if (!skinname[0]) {
+	skinname = Info_ValueForKey(sc->userinfo, "skin");
+	if (!skinname || !skinname[0])
+	    skinname = baseskin.string;
     }
+    if (strstr(skinname, "..") || skinname[0] == '.')
+	skinname = "base";
 
-    if (strstr(name, "..") || *name == '.')
-	strcpy(name, "base");
-
-    COM_StripExtension(name, name);
+    snprintf(name, sizeof(name), "%s", skinname);
+    COM_StripExtension(name);
 
     for (i = 0; i < numskins; i++) {
 	if (!strcmp(name, skins[i].name)) {
@@ -83,7 +81,7 @@ Skin_Find(player_info_t * sc)
     numskins++;
 
     memset(skin, 0, sizeof(*skin));
-    strncpy(skin->name, name, sizeof(skin->name) - 1);
+    snprintf(skin->name, sizeof(skin->name), "%s", name);
 }
 
 

common/common.c

@@ -938,11 +938,11 @@ COM_StripExtension
 ============
 */
 void
-COM_StripExtension(const char *in, char *out)
+COM_StripExtension(char *filename)
 {
-    while (*in && *in != '.')
-	*out++ = *in++;
-    *out = 0;
+    while (*filename && *filename != '.')
+	filename++;
+    *filename = 0;
 }
 
 /*

common/gl_mesh.c

@@ -334,13 +334,17 @@ GL_LoadMeshData(const model_t *model, aliashdr_t *hdr, const mtriangle_t *tris,
     char cache[MAX_QPATH];
     FILE *f;
     qboolean cached = false;
+    const char *name;
 
     //
     // look for a cached version
     //
-    sprintf(cache, "%s/glquake/", com_gamedir);
-    COM_StripExtension(model->name + strlen("progs/"), cache + strlen(cache));
-    strcat(cache, ".ms2");
+    name = COM_SkipPath(model->name);
+    snprintf(cache, sizeof(cache), "%s/glquake/%s", com_gamedir, name);
+    COM_StripExtension(cache);
+    if (strlen(cache) + strlen(".ms2") + 1 > sizeof(cache))
+	Sys_Error("%s: model pathname too long (%s)", __func__, model->name);
+    strncat(cache, ".ms2", 4);
 
     f = fopen(cache, "rb");
     if (f) {

common/gl_rmisc.c

@@ -366,7 +366,7 @@ R_TranslatePlayerSkin(int playernum)
     const entity_t *e;
 #endif
 #ifdef QW_HACK
-    char skin[512];
+    char *skin;
 #endif
 
     GL_DisableMultitexture();
@@ -379,8 +379,8 @@ R_TranslatePlayerSkin(int playernum)
     if (!player->name[0])
 	return;
 
-    strcpy(skin, Info_ValueForKey(player->userinfo, "skin"));
-    COM_StripExtension(skin, skin);
+    skin = Info_ValueForKey(player->userinfo, "skin");
+    COM_StripExtension(skin);
     if (player->skin && !strcasecmp(skin, player->skin->name))
 	player->skin = NULL;
 

include/common.h

@@ -183,7 +183,7 @@ void COM_Init(void);
 void COM_InitArgv(int argc, const char **argv);
 
 const char *COM_SkipPath(const char *pathname);
-void COM_StripExtension(const char *in, char *out);
+void COM_StripExtension(char *filename);
 void COM_FileBase(const char *in, char *out);
 void COM_DefaultExtension(char *path, const char *extension);
 int COM_CheckExtension(const char *path, const char *extn);