Re: [AD] dat/grabber and pack_fopen_chunk

[Evert Glebbeek <eglebbk@xxxxxxxxxx>]

On Sunday 22 January 2006 15:23, Evert Glebbeek wrote:
> Hmm... it seems that I can reproduce the crash in Linux by running the 
> grabber from a directory for which I do not have write permission 
(eg, /). 
> This is at least comforting because it makes it easier for me to debug 
and 
> test the problem in pack_fopen_chunk.

Ok, patch for pack_fopen_chunk attached. It tries to determine the path to 
the temporary directory, using GetTempPath in Windows and trying $TEMP, 
$TMP, /tmp and $HOME before . on other platforms. It then concatenates the 
name of the temporary file (as generated using the existing code) to that 
path and opens the file.
Tested and working in Linux, but can do with some more testing.
The code itself is pretty ugly, and I vote it be cleaned up by whomever 
feels like doing that!

Evert

Index: src/file.c
===================================================================
--- src/file.c	(revision 5662)
+++ src/file.c	(working copy)
@@ -1904,13 +1904,56 @@
 
       /* write a sub-chunk */ 
       int tmp_fd = -1;
+      char *tmp_dir = NULL;
+      char *tmp_name = NULL;
+      #ifndef HAVE_MKSTEMP
+      char* tmpnam_string;
+      #endif
 
+      #ifdef ALLEGRO_WINDOWS
+         int size;
+         int new_size = 64;
+         
+         /* Get the path of the temporary directory */
+         do {
+            size = new_size;
+            tmp_dir = realloc(tmp_dir, size);
+            new_size = GetTempPath(size, tmp_dir);
+         } while ( (size > new_size) && (new_size > 0) );
+         
+         /* Check if we retrieved the path OK */
+         if (new_size == 0)
+            sprintf(tmp_dir, "");
+      #else
+         /* Get the path of the temporary directory */
+
+         /* Try various possible locations to store the temporary file */
+         if (getenv("TEMP")) {
+            tmp_dir = strdup(getenv("TEMP"));
+         }
+         else if (getenv("TMP")) {
+            tmp_dir = strdup(getenv("TMP"));
+         }
+         else if (file_exists("/tmp", FA_DIREC, NULL)) {
+            tmp_dir = strdup("/tmp");
+         }
+         else if (getenv("HOME")) {
+            tmp_dir = strdup(getenv("HOME"));
+         }
+         else {
+            /* Give up - try current directory */
+            tmp_dir = strdup(".");
+         }
+
+      #endif
+
       /* the file is open in read/write mode, even if the pack file
        * seems to be in write only mode
        */
       #ifdef HAVE_MKSTEMP
 
-         char tmp_name[] = "XXXXXX";
+         tmp_name = malloc(strlen(tmp_dir) + 16);
+         sprintf(tmp_name, "%s/XXXXXX", tmp_dir);
          tmp_fd = mkstemp(tmp_name);
 
       #else
@@ -1918,7 +1961,13 @@
          /* note: since the filename creation and the opening are not
           * an atomic operation, this is not secure
           */
-         char *tmp_name = tmpnam(NULL);
+         tmpnam_string = tmpnam(NULL);
+         tmp_name = malloc(strlen(tmp_dir) + strlen(tmpnam_string) + 2);
+         sprintf(tmp_name, "%s/%s", tmp_dir, tmpnam_string);
+         free(tmpnam_string);
+
+         tmp_fd = mkstemp(tmp_name);
+
          if (tmp_name) {
 #ifndef ALLEGRO_MPW
             tmp_fd = open(tmp_name, O_RDWR | O_BINARY | O_CREAT | O_EXCL, OPEN_PERMS);
@@ -1929,8 +1978,12 @@
 
       #endif
 
-      if (tmp_fd < 0)
+      if (tmp_fd < 0) {
+         free(tmp_dir);
+         free(tmp_name);
+      
          return NULL;
+      }
 
       name = uconvert_ascii(tmp_name, tmp);
       chunk = _pack_fdopen(tmp_fd, (pack ? F_WRITE_PACKED : F_WRITE_NOPACK));
@@ -1945,6 +1998,9 @@
 
 	 chunk->normal.flags |= PACKFILE_FLAG_CHUNK;
       }
+      
+      free(tmp_dir);
+      free(tmp_name);
    }
    else {
       /* read a sub-chunk */