[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [26394] trunk/blender/source/blender: Fix for lack of basic error checking in writing compressed .blend files

[Brecht Van Lommel]

Revision: 26394
          http://projects.blender.org/plugins/scmsvn/viewcvs.php?view=rev&root=bf-blender&revision=26394
Author:   blendix
Date:     2010-01-29 12:26:17 +0100 (Fri, 29 Jan 2010)

Log Message:
-----------
Fix for lack of basic error checking in writing compressed .blend files
(which is enabled by default). If there was a problem reading or writing
in the compression process, the original .blend file could get lost. Now
errors are checked, and writing is done as follows:

write .blend@ -> compress .blend@ to .blend at .gz ->
rename .blend at .gz to .blend -> remove .blend@

We've had blender crash here, lose the original .blend and leave an
empty .blend at . It is not clear to me where this would happen in practice 
if there is enough disk space and permissions are correct, so the actual
crash is likely not fixed by this commit.

Modified Paths:
--------------
    trunk/blender/source/blender/blenlib/intern/fileops.c
    trunk/blender/source/blender/blenloader/intern/writefile.c

Modified: trunk/blender/source/blender/blenlib/intern/fileops.c
===================================================================
--- trunk/blender/source/blender/blenlib/intern/fileops.c	2010-01-29 09:56:32 UTC (rev 26393)
+++ trunk/blender/source/blender/blenlib/intern/fileops.c	2010-01-29 11:26:17 UTC (rev 26394)
@@ -153,29 +153,39 @@
 	char buffer[10240];
 	int file;
 	int readsize = 0;
+	int rval= 0, err;
+	gzFile gzfile;
 	
-	gzFile gzfile = gzopen(to,"wb"); 
-	if (NULL == gzfile) return -1;
+	gzfile = gzopen(to, "wb"); 
+	if(gzfile == NULL)
+		return -1;
 	
-	file = open(from,O_BINARY|O_RDONLY);
-	
-	if (file < 0)	return -2;
+	file = open(from, O_BINARY|O_RDONLY);
+	if(file < 0)
+		return -2;
 
-	while ( 1 )
-	{
+	while(1) {
 		readsize = read(file, buffer, 10240);
+
+		if(readsize < 0) {
+			rval= -2; /* error happened in reading */
+			fprintf(stderr, "Error reading file %s: %s.\n", from, strerror(errno));
+			break;
+		}
+		else if(readsize == 0)
+			break; /* done reading */
 		
-		if (readsize <= 0) break;
-		
-		gzwrite(gzfile,buffer,readsize);
+		if(gzwrite(gzfile, buffer, readsize) <= 0) {
+			rval= -1; /* error happened in writing */
+			fprintf(stderr, "Error writing gz file %s: %s.\n", to, gzerror(gzfile, &err));
+			break;
+		}
 	}
 	
 	gzclose(gzfile);
 	close(file);
-	
-	remove(from);
 
-	return 0;
+	return rval;
 }
 
 /* return 1 when file can be written */

Modified: trunk/blender/source/blender/blenloader/intern/writefile.c
===================================================================
--- trunk/blender/source/blender/blenloader/intern/writefile.c	2010-01-29 09:56:32 UTC (rev 26393)
+++ trunk/blender/source/blender/blenloader/intern/writefile.c	2010-01-29 11:26:17 UTC (rev 26394)
@@ -2445,10 +2445,11 @@
 int BLO_write_file(Main *mainvar, char *dir, int write_flags, ReportList *reports)
 {
 	char userfilename[FILE_MAXDIR+FILE_MAXFILE];
-	char tempname[FILE_MAXDIR+FILE_MAXFILE];
+	char tempname[FILE_MAXDIR+FILE_MAXFILE+1];
 	int file, err, write_user_block;
 
-	sprintf(tempname, "%s@", dir);
+	/* open temporary file, so we preserve the original in case we crash */
+	BLI_snprintf(tempname, sizeof(tempname), "%s@", dir);
 
 	file = open(tempname,O_BINARY+O_WRONLY+O_CREAT+O_TRUNC, 0666);
 	if(file == -1) {
@@ -2456,6 +2457,7 @@
 		return 0;
 	}
 
+	/* remapping of relative paths to new file location */
 	if(write_flags & G_FILE_RELATIVE_REMAP) {
 		char dir1[FILE_MAXDIR+FILE_MAXFILE];
 		char dir2[FILE_MAXDIR+FILE_MAXFILE];
@@ -2470,7 +2472,6 @@
 			write_flags &= ~G_FILE_RELATIVE_REMAP;
 		else
 			makeFilesAbsolute(G.sce, NULL);
-
 	}
 
 	BLI_make_file_string(G.sce, userfilename, BLI_gethome(), ".B25.blend");
@@ -2479,33 +2480,46 @@
 	if(write_flags & G_FILE_RELATIVE_REMAP)
 		makeFilesRelative(dir, NULL); /* note, making relative to something OTHER then G.sce */
 
+	/* actual file writing */
 	err= write_file_handle(mainvar, file, NULL,NULL, write_user_block, write_flags);
 	close(file);
 
+	/* rename/compress */
 	if(!err) {
-		if(write_flags & G_FILE_COMPRESS)
-		{	
-			// compressed files have the same ending as regular files... only from 2.4!!!
+		if(write_flags & G_FILE_COMPRESS) {
+			/* compressed files have the same ending as regular files... only from 2.4!!! */
+			char gzname[FILE_MAXDIR+FILE_MAXFILE+4];
+			int ret;
+
+			/* first write compressed to separate @.gz */
+			BLI_snprintf(gzname, sizeof(gzname), "%s at .gz", dir);
+			ret = BLI_gzip(tempname, gzname);
 			
-			int ret = BLI_gzip(tempname, dir);
-			
-			if(-1==ret) {
+			if(0==ret) {
+				/* now rename to real file name, and delete temp @ file too */
+				if(BLI_rename(gzname, dir) != 0) {
+					BKE_report(reports, RPT_ERROR, "Can't change old file. File saved with @.");
+					return 0;
+				}
+
+				BLI_delete(tempname, 0, 0);
+			}
+			else if(-1==ret) {
 				BKE_report(reports, RPT_ERROR, "Failed opening .gz file.");
 				return 0;
 			}
-			if(-2==ret) {
+			else if(-2==ret) {
 				BKE_report(reports, RPT_ERROR, "Failed opening .blend file for compression.");
 				return 0;
 			}
 		}
-		else
-		if(BLI_rename(tempname, dir) != 0) {
+		else if(BLI_rename(tempname, dir) != 0) {
 			BKE_report(reports, RPT_ERROR, "Can't change old file. File saved with @");
 			return 0;
 		}
-
 		
-	} else {
+	}
+	else {
 		BKE_report(reports, RPT_ERROR, strerror(errno));
 		remove(tempname);