On Mon, 18 Jun 2007, Paul Kelly wrote:

On Mon, 18 Jun 2007, Hamish wrote:

[r.average]

should G_tempfile() be calling G_convert_dirseps_to_host() internally,
rather than having individual modules calling it? (ie often forgetting
to call it)

I don't think so - G_convert_dirseps_to_host() only needs to be called in certain situations, when the generated path will be used directly by external commands on the system. I think to unconditionally call it would confuse its purpose. But then again in the longrun it might be a good idea to generally simplify things as you said. Hmm, I'm not so sure now. But there may be cases which call it and expect to have '/' directory separators there for some further processing; I wouldn't like to just change it without an audit of where it is used. Which modules were you thinking of anyway?

FWIW I've attached my proposed changes to lib/gis/tempfile.c that I still
            ^^^^^^^^
Oops - didn't even upload the diff never mind attach it. But here it is now if anyone is interested. Interestingly, the patch forces all generated filenames to have '/' directory separators - presumably because I was concerned that some modules might expect this, although I can't remember to be sure...

have in my local source tree. They were intended to split the usage of G_tempfile() between modules that are genuinely using it as a tempfile for creating large temporary maps as was originally intended, and those that are just using it as a general small tempfile function. They also make some changes to make it more Windows compatible. But we decided they were too controversial to commit in case they broke things - maybe in some way we can eventually merge them though.

Paul

_______________________________________________
grass-dev mailing list
[email protected]
http://grass.itc.it/mailman/listinfo/grass-dev

Index: lib/gis/tempfile.c

===================================================================

RCS file: /home/grass/grassrepository/grass6/lib/gis/tempfile.c,v

retrieving revision 2.5

diff -u -r2.5 tempfile.c

--- lib/gis/tempfile.c  14 Apr 2007 23:02:01 -0000      2.5

+++ lib/gis/tempfile.c  18 Jun 2007 09:17:45 -0000

@@ -11,36 +11,62 @@

  * \date 1999-2006

  */

 

+#include <stdio.h>

+#include <stdlib.h>

 #include <string.h>

 #include <unistd.h>

 #include <sys/stat.h>

+#include <sys/types.h>

 #include <grass/gis.h>

 

-

-/**

- * \fn char *G_tempfile (void)

+/*!

+ * \brief returns a temporary file name on the same disk as the current mapset

  *

- * \brief Returns a temporary file name.

+ * This function returns a pointer to a string containing a unique file name 

+ * that can be used as a temporary file within the module. Successive calls 

+ * to the function will generate new names.

+ * 

+ * Only the file name is generated. The file itself is not created. To create 
the

+ * file, the module must use standard UNIX functions which create and open 
files,

+ * e.g., creat() or fopen(). The filename is guaranteed to be on the same disk

+ * as the current mapset. This means that "draft" output maps can

+ * be moved (with either link()+remove() or rename()) into their final

+ * location upon closure.

+ * 

+ * The programmer should take reasonable care to remove (unlink) the file 

+ * before the module exits. However, GRASS database management will 

+ * eventually remove all temporary files created by G_tempfile_in_mapset() 

+ * that have been left behind by the modules which created them.

+ *

+ *  \return char:  pointer to a character string containing the name.

+ *   the name is copied to allocated memory and may be

+ *   released by the unix free () routine.

+ */

+

+char *G_tempfile_in_mapset(void)

+{

+    return G__tempfile_in_mapset(getpid());

+}

+

+/*!

+ * \brief returns a temporary file name

  *

- * This routine returns a pointer to a string containing a unique 

- * temporary file name that can be used as a temporary file within the 

- * module. Successive calls to <i>G_tempfile()</i> will generate new 

- * names. Only the file name is generated. The file itself is not 

- * created. To create the file, the module must use standard UNIX 

- * functions which create and open files, e.g., <i>creat()</i> or 

- * <i>fopen()</i>.<br>

- *

- * Successive calls will generate different names the names are of the 

- * form pid.n where pid is the programs process id number and n is a 

- * unique identifier.<br>

- *

- * <b>Note:</b> It is recommended to <i>unlink()</i> (remove) the 

- * temp file on exit/error. Only if GRASS is left with 'exit', the GIS 

- * mapset manangement will clean up the temp directory (ETC/clean_temp).

- *

- * \return pointer to a character string containing the name. The name 

- * is copied to allocated memory and may be released by the unix free() 

- * routine.

+ * This function returns a pointer to a string containing a unique file name 

+ * that can be used as a temporary file within the module. Successive calls 

+ * to the function will generate new names.

+ * 

+ * Only the file name is generated. The file itself is not created. To create 
the

+ * file, the module must use standard UNIX functions which create and open 
files,

+ * e.g., creat() or fopen().

+ * 

+ * The programmer should take care to remove (unlink) the file before

+ * the module exits. If a module requires not to immediately delete a 

+ * temporary file and instead have it automatically deleted at the end of

+ * the GRASS session, G_tempfile_in_mapset() should be used instead.

+ *

+ *  \return char:  pointer to a character string containing the name.

+ *   the name is copied to allocated memory and may be

+ *   released by the unix free () routine.

  */

 

 char *G_tempfile(void)

@@ -48,10 +74,7 @@

     return G__tempfile(getpid());

 }

 

-

 /**

- * \fn char *G__tempfile (int pid)

- *

  * \brief Create tempfile from process id.

  *

  * See <i>G_tempfile()</i>.

@@ -62,6 +85,65 @@

 

 char *G__tempfile (int pid)

 {

+    char *env_tmpdir = getenv("TMPDIR");

+    char *env_temp = getenv("TEMP");

+    char *prefix, *path = NULL;

+    char dirsep_char[2];       

+    static int uniq = 0;

+    struct stat st;

+   

+    if (pid <= 0)

+       pid = getpid();

+   

+    if (env_tmpdir != NULL)

+       /* This should be somewhere in user home directory on Unix,

+        * thus ensuring write permission */

+        prefix = G_store(env_tmpdir);

+    else if (env_temp != NULL)

+       /* This should be somewhere in user profile directory on Windows, 

+        * thus ensuring write permission */

+        prefix = G_store(env_temp);

+    else

+        /* Usually /tmp on Unix or root of current drive on Windows */

+        prefix = G_store(P_tmpdir);

+

+    /* Often there's no harm in having an extra directory separator character

+     * in the middle of a path but just being careful here. */

+    if (G_is_dirsep( prefix[strlen(prefix)-1] )) {            

+       dirsep_char[0] = '\0';

+    }      

+    else {            

+       dirsep_char[0] = GRASS_DIRSEP;

+       dirsep_char[1] = '\0';

+    }

+           

+    do {

+        if (path != NULL)

+           G_free(path);

+        path = G_malloc(strlen(prefix) + 40 );

+        sprintf(path, "%s%sgrass-%d.%d", prefix, dirsep_char, pid, uniq++);

+        /* We use '/'-style dirseps within GRASS */

+        G_convert_dirseps_from_host( path );

+    } while (stat(path, &st) == 0);

+

+    G_free(prefix);

+   

+    return path;

+   

+}

+

+

+/**

+ * \brief Create tempfile in current mapset from process id.

+ *

+ * See <i>G_tempfile_in_mapset()</i>.

+ *

+ * \param[in] pid

+ * \return Pointer to string path

+ */

+

+char *G__tempfile_in_mapset (int pid)

+{

     char path[GPATH_MAX];

     char name[GNAME_MAX];

     char element[100];

@@ -78,13 +160,14 @@

     }

     while (stat(path, &st) == 0) ;

 

+    /* We use '/'-style dirseps within GRASS */

+    G_convert_dirseps_from_host (path);

+   

     return G_store (path);

 }

 

 

 /**

- * \fn int G__temp_element (char *element)

- *

  * \brief Populates <b>element</b> with a path string.

  *

  * \param[in,out] element

_______________________________________________
grass-dev mailing list
[email protected]
http://grass.itc.it/mailman/listinfo/grass-dev

Reply via email to