[quakefs] Return QFile pointer from QFS_NextFile(name)

QFS_NextFilename was renamed to QFS_NextFile to reflect the fact it now
returns a QFile pointer for the newly created file (as well as the
name). This necessitated updating WritePNG to take a file pointer
instead of a file name, with the advantage that WritePNGqfs is no longer
necessary and callers have much more control over the creation of the
file.

This makes QFS_NextFile much more secure against file system race
conditions and attacks (at least in theory). If nothing else, it will
make it more robust in a multi-threaded environment.

include/QF/png.h

@@ -34,8 +34,6 @@
 #include "QF/quakefs.h"
 
 struct tex_s *LoadPNG (QFile *infile, int load);
-void WritePNG (const char *fileName, const byte *data, int width, int height);
-void WritePNGqfs (const char *fileName, const byte *data,
-				  int width, int height);
+int WritePNG (QFile *outfile, const byte *data, int width, int height);
 
 #endif//__QF_png_h

include/QF/quakefs.h

@@ -290,17 +290,20 @@ int QFS_Remove (const char *path);
 /**	Find available filename.
 
 	The filename will be of the form \c prefixXXXX.ext where \c XXXX
-	is a zero padded number from 0 to 9999.
+	is a zero padded number from 0 to 9999. Should there already be 10000
+	files of such a pattern, then extra digits will be added.
 
 	\param filename	This will be set to the available filename.
 	\param prefix	The part of the filename preceeding the numers.
 	\param ext		The extension to add to the filename.
-	\return			1 for success, 0 for failure.
+	\return			NULL for failure (with an error message in \a filename)
+					or a quakeio file handle.
 
-	\note \a prefix is relative to \c qfc_userpath.
+	\note \a prefix is relative to \c qfc_userpath, as is the generated
+	filename.
 */
-int QFS_NextFilename (struct dstring_s *filename, const char *prefix,
-					  const char *ext);
+QFile *QFS_NextFile (struct dstring_s *filename, const char *prefix,
+					 const char *ext);
 
 /** Extract the non-extension part of the file name from the path.
 

libs/image/png.c

@@ -205,8 +205,8 @@ LoadPNG (QFile *infile, int load)
 
 #define WRITEPNG_BIT_DEPTH 8
 
-static int
-write_png (QFile *outfile, const byte *data, int width, int height)
+int
+WritePNG (QFile *outfile, const byte *data, int width, int height)
 {
 	int         i;
 	png_structp png_ptr;
@@ -282,36 +282,6 @@ write_png (QFile *outfile, const byte *data, int width, int height)
 	return 1;
 }
 
-VISIBLE void
-WritePNG (const char *fileName, const byte *data, int width, int height)
-{
-	QFile      *outfile;
-
-	outfile = Qopen (fileName, "wb");
-	if (!outfile) {
-		Sys_Printf ("Couldn't open %s\n", fileName);
-		return; /* Can't open file */
-	}
-	if (!write_png (outfile, data, width, height))
-		Qremove (fileName);
-	Qclose (outfile);
-}
-
-VISIBLE void
-WritePNGqfs (const char *fileName, const byte *data, int width, int height)
-{
-	QFile      *outfile;
-
-	outfile = QFS_Open (fileName, "wb");
-	if (!outfile) {
-		Sys_Printf ("Couldn't open %s\n", fileName);
-		return; /* Can't open file */
-	}
-	if (!write_png (outfile, data, width, height))
-		QFS_Remove (fileName);
-	Qclose (outfile);
-}
-
 #else
 
 #include "QF/image.h"
@@ -324,12 +294,7 @@ LoadPNG (QFile *infile, int load)
 }
 
 VISIBLE void
-WritePNG (const char *fileName, const byte *data, int width, int height)
-{
-}
-
-VISIBLE void
-WritePNGqfs (const char *fileName, const byte *data, int width, int height)
+WritePNG (QFile *outfile, const byte *data, int width, int height)
 {
 }
 

libs/util/quakefs.c

@@ -1554,10 +1554,10 @@ QFS_SetExtension (struct dstring_s *path, const char *extension)
 	dstring_appendstr (path, extension);
 }
 
-VISIBLE int
-QFS_NextFilename (dstring_t *filename, const char *prefix, const char *ext)
+VISIBLE QFile *
+QFS_NextFile (dstring_t *filename, const char *prefix, const char *ext)
 {
-	int         ret = 0;
+	QFile      *file = 0;
 	dstring_t  *full_path = dstring_new ();
 
 	if (qfs_expand_userpath (full_path, "") == -1) {
@@ -1568,12 +1568,13 @@ QFS_NextFilename (dstring_t *filename, const char *prefix, const char *ext)
 		int         fd = Sys_UniqueFile (filename, full_path->str, ext, 4);
 		if (fd >= 0) {
 			dstring_snip (filename, 0, qfs_pos);
-			close (fd);
-			ret = 1;
+			// Sys_UniqueFile opens with O_RDWR, and ensure binary files work
+			// on Windows. gzip writing is NOT specified
+			file = Qdopen (fd, "w+b");
 		}
 	}
 	dstring_delete (full_path);
-	return ret;
+	return file;
 }
 
 VISIBLE QFile *

libs/video/renderer/r_screen.c

@@ -361,18 +361,20 @@ static void
 ScreenShot_f (void)
 {
 	dstring_t  *name = dstring_new ();
+	QFile      *file;
 
 	// find a file name to save it to
-	if (!QFS_NextFilename (name, va (0, "%s/qf",
-									 qfs_gamedir->dir.shots), ".png")) {
+	if (!(file = QFS_NextFile (name, va (0, "%s/qf", qfs_gamedir->dir.shots),
+							   ".png"))) {
 		Sys_Printf ("SCR_ScreenShot_f: Couldn't create a PNG file: %s\n",
 					name->str);
 	} else {
 		tex_t      *tex;
 
 		tex = r_funcs->SCR_CaptureBGR ();
-		WritePNGqfs (name->str, tex->data, tex->width, tex->height);
+		WritePNG (file, tex->data, tex->width, tex->height);
 		free (tex);
+		Qclose (file);
 		Sys_Printf ("Wrote %s/%s\n", qfs_userpath, name->str);
 	}
 	dstring_delete (name);

libs/video/renderer/vid_render_vulkan.c

@@ -462,17 +462,18 @@ vulkan_set_fov (float x, float y)
 
 	mctx->dirty = mctx->frames.size;
 }
-
+#if 0
 static int
 is_bgr (VkFormat format)
 {
 	return (format >= VK_FORMAT_B8G8R8A8_UNORM
 			&& format <= VK_FORMAT_B8G8R8A8_SRGB);
 }
-
+#endif
 static void
 capture_screenshot (const byte *data, int width, int height)
 {
+#if 0
 	dstring_t  *name = dstring_new ();
 	// find a file name to save it to
 	if (!QFS_NextFilename (name, va (vulkan_ctx->va_ctx, "%s/qf",
@@ -502,6 +503,7 @@ capture_screenshot (const byte *data, int width, int height)
 		}
 	}
 	dstring_delete (name);
+#endif
 }
 
 static tex_t *

nq/source/host.c

@@ -702,9 +702,13 @@ _Host_Frame (float time)
 
 	if (cls.demo_capture) {
 		tex_t      *tex = r_funcs->SCR_CaptureBGR ();
-		WritePNGqfs (va (0, "%s/qfmv%06d.png", qfs_gamedir->dir.shots,
-						 cls.demo_capture++),
-					 tex->data, tex->width, tex->height);
+		QFile      *file = Qopen (va (0, "%s/qfmv%06d.png",
+									  qfs_gamedir->dir.shots,
+									  cls.demo_capture++), "wb");
+		if (file) {
+			WritePNG (file, tex->data, tex->width, tex->height);
+			Qclose (file);
+		}
 		free (tex);
 	}
 

qw/include/server.h

@@ -252,6 +252,7 @@ typedef struct client_s {
 
 	QFile			*upload;
 	struct dstring_s *uploadfn;
+	int             upload_started;
 	netadr_t		snap_from;
 	qboolean		remote_snap;
 

qw/source/cl_main.c

@@ -1752,9 +1752,13 @@ Host_Frame (float time)
 
 	if (cls.demo_capture) {
 		tex_t      *tex = r_funcs->SCR_CaptureBGR ();
-		WritePNGqfs (va (0, "%s/qfmv%06d.png", qfs_gamedir->dir.shots,
-						 cls.demo_capture++),
-					 tex->data, tex->width, tex->height);
+		QFile      *file = Qopen (va (0, "%s/qfmv%06d.png",
+								  qfs_gamedir->dir.shots, cls.demo_capture++),
+								  "wb");
+		if (file) {
+			WritePNG (file, tex->data, tex->width, tex->height);
+			Qclose (file);
+		}
 		free (tex);
 	}
 

qw/source/sv_ccmds.c

@@ -249,13 +249,12 @@ SV_Fraglogfile_f (void)
 		return;
 	}
 	name = dstring_new ();
-	if (!QFS_NextFilename (name,
-						   va (0, "%s/frag_", qfs_gamedir->dir.def), ".log")) {
-		SV_Printf ("Can't open any logfiles.\n");
-		sv_fraglogfile = NULL;
+	if (!(sv_fraglogfile = QFS_NextFile (name,
+										 va (0, "%s/frag_",
+											 qfs_gamedir->dir.def), ".log"))) {
+		SV_Printf ("Can't open any logfiles.\n%s\n", name->str);
 	} else {
 		SV_Printf ("Logging frags to %s.\n", name->str);
-		sv_fraglogfile = QFS_WOpen (name->str, 0);
 	}
 	dstring_delete (name);
 }
@@ -1136,14 +1135,16 @@ SV_Snap (int uid)
 	if (!cl->uploadfn)
 		cl->uploadfn = dstring_new ();
 
-	if (!QFS_NextFilename (cl->uploadfn,
-						   va (0, "%s/snap/%d-", qfs_gamedir->dir.def, uid),
-						   ".pcx")) {
-		SV_Printf ("Snap: Couldn't create a file, clean some out.\n");
+	if (!(cl->upload = QFS_NextFile (cl->uploadfn,
+									 va (0, "%s/snap/%d-",
+										 qfs_gamedir->dir.def, uid), ".pcx"))) {
+		SV_Printf ("Snap: Couldn't create a file, clean some out.\n%s\n",
+				   cl->uploadfn->str);
 		dstring_delete (cl->uploadfn);
 		cl->uploadfn = 0;
 		return;
 	}
+	cl->upload_started = 0;
 
 	memcpy (&cl->snap_from, &net_from, sizeof (net_from));
 	if (sv_redirected != RD_NONE)

qw/source/sv_demo.c

@@ -366,12 +366,6 @@ SV_Record (char *name)
 	client_t   *player;
 	const char *gamedir, *s;
 
-	demo_file = QFS_Open (name, "wb");
-	if (!demo_file) {
-		Sys_Printf ("ERROR: couldn't open %s\n", name);
-		return;
-	}
-
 	SV_InitRecord ();
 
 	dstring_copystr (demo_name, name);
@@ -642,7 +636,12 @@ SV_Record_f (void)
 	// open the demo file
 	QFS_DefaultExtension (name, ".mvd");
 
-	SV_Record (name->str);
+	demo_file = QFS_Open (name->str, "wb");
+	if (!demo_file) {
+		Sys_Printf ("ERROR: couldn't open %s\n", name->str);
+	} else {
+		SV_Record (name->str);
+	}
 
 	dstring_delete (name);
 }
@@ -757,8 +756,9 @@ SV_EasyRecord_f (void)
 			  sv_demoPrefix->string, SV_CleanName (name->str),
 			  sv_demoSuffix->string);
 
-	if (QFS_NextFilename (name, name2->str, ".mvd"))
+	if ((demo_file = QFS_NextFile (name, name2->str, ".mvd"))) {
 		SV_Record (name->str);
+	}
 
 	dstring_delete (name);
 	dstring_delete (name2);

qw/source/sv_user.c

@@ -647,7 +647,7 @@ SV_NextUpload (void)
 {
 	int		percent, size;
 
-	if (!host_client->uploadfn) {
+	if (!host_client->upload) {
 		SV_ClientPrintf (1, host_client, PRINT_HIGH, "Upload denied\n");
 		MSG_ReliableWrite_Begin (&host_client->backbuf, svc_stufftext, 8);
 		MSG_ReliableWrite_String (&host_client->backbuf, "stopul");
@@ -662,16 +662,8 @@ SV_NextUpload (void)
 	size = MSG_ReadShort (net_message);
 	percent = MSG_ReadByte (net_message);
 
-	if (!host_client->upload) {
-		host_client->upload = QFS_Open (host_client->uploadfn->str, "wb");
-		if (!host_client->upload) {
-			SV_Printf ("Can't create %s\n", host_client->uploadfn->str);
-			MSG_ReliableWrite_Begin (&host_client->backbuf, svc_stufftext, 8);
-			MSG_ReliableWrite_String (&host_client->backbuf, "stopul");
-			dstring_delete (host_client->uploadfn);
-			host_client->uploadfn = 0;
-			return;
-		}
+	if (!host_client->upload_started) {
+		host_client->upload_started = 1;
 		SV_Printf ("Receiving %s from %d...\n", host_client->uploadfn->str,
 					host_client->userid);
 		if (host_client->remote_snap)
@@ -708,6 +700,7 @@ SV_NextUpload (void)
 		}
 		dstring_delete (host_client->uploadfn);
 		host_client->uploadfn = 0;
+		host_client->upload_started = 0;
 	}
 
 }

tools/bsp2img/bsp2img.c

@@ -898,7 +898,11 @@ write_png (image_t *image)
 		*out++ = b;
 		*out++ = b;
 	}
-	WritePNG (options.outf_name, data, image->width, image->height);
+	QFile      *file = Qopen (options.outf_name, "wb");
+	if (file) {
+		WritePNG (file, data, image->width, image->height);
+		Qclose (file);
+	}
 }
 
 static void