Get rid of set_default_file_name()
authorTeemu Murtola <teemu.murtola@gmail.com>
Mon, 16 Dec 2013 16:18:22 +0000 (18:18 +0200)
committerTeemu Murtola <teemu.murtola@gmail.com>
Tue, 24 Dec 2013 04:38:20 +0000 (06:38 +0200)
A global variable protected by a mutex is not actually required to get
the same functionality: the default value can be simply passed to
parse_file_args(), and propagated where needed.  This makes the design
properly thread-safe, allows separation of file type handling from file
argument handling (not done here), and simplifies the logic by removing
confusing bCanNotOverride parameters (replaced by passing the default
file name only where it belongs).

Also, clean up includes in filenm.*.

Change-Id: I831576963b40f9e0a8bbaa741c80c3e59ec6f85c

src/gromacs/commandline/pargs.cpp
src/gromacs/fileio/filenm.c
src/gromacs/fileio/filenm.h

index cd4a1e05ea4a2140adc9bd9ed3f6a99215fa330e..757dfc51cced632d3f0ca9dc03e7ff5a6168705a 100644 (file)
@@ -765,13 +765,9 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags,
     output_env_init(oenv, *argc, argv, (time_unit_t)nenum(time_units), bView,
                     (xvg_format_t)nenum(xvg_format), 0, debug_level);
 
-    if (FF(PCA_CAN_SET_DEFFNM) && (deffnm != NULL))
-    {
-        set_default_file_name(deffnm);
-    }
-
     /* Parse the file args */
-    parse_file_args(argc, argv, nfile, fnm, FF(PCA_KEEP_ARGS), !FF(PCA_NOT_READ_NODE));
+    parse_file_args(argc, argv, nfile, fnm, deffnm,
+                    FF(PCA_KEEP_ARGS), !FF(PCA_NOT_READ_NODE));
 
     /* Open the debug file */
     if (debug_level > 0)
index e439b0fe2aef4fb7bfff3e9e3ad52bbeba8fc827..688ead16e6efbc56c6f1e428b64c1e4445be70d1 100644 (file)
 #include <config.h>
 #endif
 
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
-#include "sysstuff.h"
-#include "typedefs.h"
-#include "smalloc.h"
-#include "string2.h"
-#include "gmx_fatal.h"
+
 #include "futil.h"
-#include "xdrf.h"
+#include "gmx_fatal.h"
 #include "macros.h"
-
-#include "gromacs/legacyheaders/thread_mpi/threads.h"
-
-/* NOTE: this was a cesspool of thread-unsafe code, has now been
-   properly proteced by mutexes (hopefully). */
+#include "smalloc.h"
+#include "string2.h"
+#include "types/simple.h"
+#include "types/commrec.h"
 
 /* XDR should be available on all platforms now,
  * but we keep the possibility of turning it off...
@@ -205,21 +202,10 @@ static const t_deffile
     { eftASC, "", "rundir", NULL, "Run directory" }
 };
 
-static char *default_file_name = NULL;
-
-static tMPI_Thread_mutex_t filenm_mutex = TMPI_THREAD_MUTEX_INITIALIZER;
-
 #define NZEXT 2
 static const char *z_ext[NZEXT] =
 { ".gz", ".Z" };
 
-void set_default_file_name(const char *name)
-{
-    tMPI_Thread_mutex_lock(&filenm_mutex);
-    default_file_name = strdup(name);
-    tMPI_Thread_mutex_unlock(&filenm_mutex);
-}
-
 const char *ftp2ext(int ftp)
 {
     if ((0 <= ftp) && (ftp < efNR))
@@ -332,25 +318,14 @@ const char *ftp2ftype(int ftp)
 
 const char *ftp2defnm(int ftp)
 {
-    const char *buf = NULL;
-
-    tMPI_Thread_mutex_lock(&filenm_mutex);
-
-    if (default_file_name)
+    if ((0 <= ftp) && (ftp < efNR))
     {
-        buf = default_file_name;
+        return deffile[ftp].defnm;
     }
     else
     {
-        if ((0 <= ftp) && (ftp < efNR))
-        {
-            buf = deffile[ftp].defnm;
-        }
+        return NULL;
     }
-
-    tMPI_Thread_mutex_unlock(&filenm_mutex);
-
-    return buf;
 }
 
 static void check_opts(int nf, t_filenm fnm[])
@@ -433,7 +408,7 @@ static void add_filenm(t_filenm *fnm, const char *filenm)
     fnm->nfiles++;
 }
 
-static void set_grpfnm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride)
+static void set_grpfnm(t_filenm *fnm, const char *name, const char *deffnm)
 {
     char       buf[256], buf2[256];
     int        i, type;
@@ -449,7 +424,7 @@ static void set_grpfnm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride
     }
 
     bValidExt = FALSE;
-    if (name && (bCanNotOverride || (default_file_name == NULL)))
+    if (name && deffnm == NULL)
     {
         strcpy(buf, name);
         /* First check whether we have a valid filename already */
@@ -467,6 +442,10 @@ static void set_grpfnm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride
             }
         }
     }
+    else if (deffnm != NULL)
+    {
+        strcpy(buf, deffnm);
+    }
     else
     {
         /* No name given, set the default name */
@@ -498,7 +477,7 @@ static void set_grpfnm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride
     add_filenm(fnm, buf);
 }
 
-static void set_filenm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride,
+static void set_filenm(t_filenm *fnm, const char *name, const char *deffnm,
                        gmx_bool bReadNode)
 {
     /* Set the default filename, extension and option for those fields that
@@ -542,14 +521,20 @@ static void set_filenm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride
 
     if (deffile[fnm->ftp].ntps)
     {
-        set_grpfnm(fnm, name ? buf : NULL, bCanNotOverride);
+        set_grpfnm(fnm, name ? buf : NULL, deffnm);
     }
     else
     {
-        if ((name == NULL) || !(bCanNotOverride || (default_file_name == NULL)))
+        if (name == NULL || deffnm != NULL)
         {
-            const char *defnm = ftp2defnm(fnm->ftp);
-            strcpy(buf, defnm);
+            if (deffnm != NULL)
+            {
+                strcpy(buf, deffnm);
+            }
+            else
+            {
+                strcpy(buf, ftp2defnm(fnm->ftp));
+            }
         }
         set_extension(buf, fnm->ftp);
 
@@ -557,7 +542,7 @@ static void set_filenm(t_filenm *fnm, const char *name, gmx_bool bCanNotOverride
     }
 }
 
-static void set_filenms(int nf, t_filenm fnm[], gmx_bool bReadNode)
+static void set_filenms(int nf, t_filenm fnm[], const char *deffnm, gmx_bool bReadNode)
 {
     int i;
 
@@ -565,13 +550,13 @@ static void set_filenms(int nf, t_filenm fnm[], gmx_bool bReadNode)
     {
         if (!IS_SET(fnm[i]))
         {
-            set_filenm(&(fnm[i]), fnm[i].fn, FALSE, bReadNode);
+            set_filenm(&(fnm[i]), fnm[i].fn, deffnm, bReadNode);
         }
     }
 }
 
 void parse_file_args(int *argc, char *argv[], int nf, t_filenm fnm[],
-                     gmx_bool bKeep, gmx_bool bReadNode)
+                     const char *deffnm, gmx_bool bKeep, gmx_bool bReadNode)
 {
     int       i, j;
     gmx_bool *bRemove;
@@ -599,12 +584,12 @@ void parse_file_args(int *argc, char *argv[], int nf, t_filenm fnm[],
                     /* check if we are out of arguments for this option */
                     if ((i >= *argc) || (argv[i][0] == '-'))
                     {
-                        set_filenm(&fnm[j], fnm[j].fn, FALSE, bReadNode);
+                        set_filenm(&fnm[j], fnm[j].fn, deffnm, bReadNode);
                     }
                     /* sweep up all file arguments for this option */
                     while ((i < *argc) && (argv[i][0] != '-'))
                     {
-                        set_filenm(&fnm[j], argv[i], TRUE, bReadNode);
+                        set_filenm(&fnm[j], argv[i], NULL, bReadNode);
                         bRemove[i] = TRUE;
                         i++;
                         /* only repeat for 'multiple' file options: */
@@ -640,7 +625,7 @@ void parse_file_args(int *argc, char *argv[], int nf, t_filenm fnm[],
         sfree(bRemove);
     }
 
-    set_filenms(nf, fnm, bReadNode);
+    set_filenms(nf, fnm, deffnm, bReadNode);
 
 }
 
index e2af5fce3fe71b1bcdcec01d41c2258fc5afe9ca..fea0ff9955d593d18edc8e59743e43bc112dcda4 100644 (file)
@@ -38,7 +38,8 @@
 #ifndef GMX_FILEIO_FILENM_H
 #define GMX_FILEIO_FILENM_H
 
-#include "futil.h"
+#include "../legacyheaders/types/simple.h"
+#include "../legacyheaders/types/commrec.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -92,9 +93,6 @@ typedef struct {
 #define ffWRMULT   (ffWRITE  | ffMULT)
 #define ffOPTWRMULT   (ffWRMULT | ffOPT)
 
-void set_default_file_name(const char *name);
-/* Set the default file name for all file types to name */
-
 const char *ftp2ext(int ftp);
 /* Return extension for filetype */
 
@@ -121,7 +119,7 @@ const char *ftp2ftype(int ftp);
 /* Return Binary or ASCII depending on file type */
 
 void parse_file_args(int *argc, char *argv[], int nf, t_filenm fnm[],
-                     gmx_bool bKeep, gmx_bool bReadNode);
+                     const char *deffnm, gmx_bool bKeep, gmx_bool bReadNode);
 /* Parse command line for file names. When bKeep is set args are
  * not removed from argv. */