More gmxfio simplification

- Remove gmx_fio_setdebug() and all implementation related to it;
  getting the source code location printed to stderr is not really
  useful for errors that typically originate from the file system.
- Move gmx_fio_setprecision() to gmxfio-xdr, since it logically belongs
  there.
- Remove bOpen, as the only case where any code could observe
  bOpen==FALSE requires a use-after-free, and possibly also an ugly
  race condition on top of that.
- Reduce use of iFTP member within gmxfio, where the purpose is to check
  whether the fio->xdr member is not NULL.
- Remove checks from gmx_fio_open() that overlapped with those in
  gmx_ffopen().  There are some corner cases that now work differently:
  gmxfio no longer bypasses the check for .Z/.gz files for xdr files,
  but lets things through to gmx_ffopen(), and with GMX_FAHCORE there is
  no longer any kind of existence check for xdr files (it is
  questionable whether the previous behavior was intentionally different
  for xdr and non-xdr files, anyways).
- Remove dead code: fio->fp is always non-NULL if fio->xdr is.

Change-Id: Icd1588bfbb510543a55f57fb86e0e1e2a1fce236

diff --git a/src/contrib/anaf.c b/src/contrib/anaf.c
index 89971946ac027ee5228a5ba559b1ac2c575d25e3..044d987048438025bd5420b110705ca1396d8f03 100644 (file)
--- a/src/contrib/anaf.c
+++ b/src/contrib/anaf.c
@@ -70,7 +70,6 @@ static void list_trn(char *fn)
   printf("Going to open %s\n",fn);
   fpread  = open_trn(fn,"r"); 
   fpwrite = open_tpx(NULL,"w");
-  gmx_fio_setdebug(fpwrite,TRUE);
   
   mmm=mass[0]+2*mass[1];
   for(i=0; (i<5); i++) 

diff --git a/src/gromacs/fileio/gmxfio-impl.h b/src/gromacs/fileio/gmxfio-impl.h
index f42facdc52b2c9a29e685acefe7bfd92dce9b444..0b455979dd6fb23c18c77202b81a6fe2e97349c6 100644 (file)
--- a/src/gromacs/fileio/gmxfio-impl.h
+++ b/src/gromacs/fileio/gmxfio-impl.h
@@ -61,10 +61,8 @@
 struct t_fileio
 {
     FILE           *fp;                /* the file pointer */
-    gmx_bool        bOpen,             /* the file is open */
-                    bRead,             /* the file is open for reading */
+    gmx_bool        bRead,             /* the file is open for reading */
                     bDouble,           /* write doubles instead of floats */
-                    bDebug,            /* the file ops should come with debug info */
                     bReadWrite;        /* the file is open for reading and writing */
     char        *fn;                   /* the file name */
     XDR         *xdr;                  /* the xdr data pointer */

diff --git a/src/gromacs/fileio/gmxfio-xdr.cpp b/src/gromacs/fileio/gmxfio-xdr.cpp
index 2c6f98fef3acc8945cfb6854ebdbc31a77315a77..b2876d0566d8e9cc738334e57e39bbbfdb9cf387 100644 (file)
--- a/src/gromacs/fileio/gmxfio-xdr.cpp
+++ b/src/gromacs/fileio/gmxfio-xdr.cpp
@@ -62,6 +62,13 @@ static const char *eioNames[eioNR] =
     "IVEC", "STRING"
 };
 
+void gmx_fio_setprecision(t_fileio *fio, gmx_bool bDouble)
+{
+    gmx_fio_lock(fio);
+    fio->bDouble = bDouble;
+    gmx_fio_unlock(fio);
+}
+
 XDR *gmx_fio_getxdr(t_fileio *fio)
 {
     XDR *ret = NULL;
@@ -336,11 +343,6 @@ static gmx_bool do_xdr(t_fileio *fio, void *item, int nitem, int eio,
         default:
             gmx_fio_fe(fio, eio, desc, srcfile, line);
     }
-    if ((res == 0) && (fio->bDebug))
-    {
-        fprintf(stderr, "Error in xdr I/O %s %s to file %s (source %s, line %d)\n",
-                eioNames[eio], desc, fio->fn, srcfile, line);
-    }
 
     return (res != 0);
 }

diff --git a/src/gromacs/fileio/gmxfio-xdr.h b/src/gromacs/fileio/gmxfio-xdr.h
index 17c3fbc60b822bd5b3d5c53935e293180c981e6d..982724d64fe4deeffafd0cc561531b5d832d81c2 100644 (file)
--- a/src/gromacs/fileio/gmxfio-xdr.h
+++ b/src/gromacs/fileio/gmxfio-xdr.h
@@ -48,6 +48,9 @@ extern "C" {
 
 struct t_fileio;
 
+void gmx_fio_setprecision(struct t_fileio *fio, gmx_bool bDouble);
+/* Select the floating point precision for reading and writing files */
+
 XDR *gmx_fio_getxdr(struct t_fileio *fio);
 /* Return the file pointer itself */
 

diff --git a/src/gromacs/fileio/gmxfio.cpp b/src/gromacs/fileio/gmxfio.cpp
index 67ab811570f8f3f70d05c6b937ed07c54d9bc691..a094435c73495cb42ddd81f823155c74d2c6a155 100644 (file)
--- a/src/gromacs/fileio/gmxfio.cpp
+++ b/src/gromacs/fileio/gmxfio.cpp
@@ -95,10 +95,6 @@ static int gmx_fio_int_flush(t_fileio* fio)
     {
         rc = fflush(fio->fp);
     }
-    else if (fio->xdr)
-    {
-        rc = fflush((FILE *) fio->xdr->x_private);
-    }
 
     return rc;
 }
@@ -309,11 +305,7 @@ t_fileio *gmx_fio_open(const char *fn, const char *mode)
     /* Check if it should be opened as a binary file */
     if (!ftp_is_text(fn2ftp(fn)))
     {
-        /* Not ascii, add b to file mode */
-        if ((strchr(newmode, 'b') == NULL) && (strchr(newmode, 'B') == NULL))
-        {
-            strcat(newmode, "b");
-        }
+        strcat(newmode, "b");
     }
 
     snew(fio, 1);
@@ -331,30 +323,10 @@ t_fileio *gmx_fio_open(const char *fn, const char *mode)
         fio->iFTP   = fn2ftp(fn);
         fio->fn     = gmx_strdup(fn);
 
+        fio->fp = gmx_ffopen(fn, newmode);
         /* If this file type is in the list of XDR files, open it like that */
         if (ftp_is_xdr(fio->iFTP))
         {
-            /* First check whether we have to make a backup,
-             * only for writing, not for read or append.
-             */
-            if (newmode[0] == 'w')
-            {
-#ifndef GMX_FAHCORE
-                /* only make backups for normal gromacs */
-                make_backup(fn);
-#endif
-            }
-            else
-            {
-                /* Check whether file exists */
-                if (!gmx_fexist(fn))
-                {
-                    gmx_open(fn);
-                }
-            }
-            /* Open the file */
-            fio->fp = gmx_ffopen(fn, newmode);
-
             /* determine the XDR direction */
             if (newmode[0] == 'w' || newmode[0] == 'a')
             {
@@ -364,15 +336,9 @@ t_fileio *gmx_fio_open(const char *fn, const char *mode)
             {
                 fio->xdrmode = XDR_DECODE;
             }
-
             snew(fio->xdr, 1);
             xdrstdio_create(fio->xdr, fio->fp, fio->xdrmode);
         }
-        else
-        {
-            /* If it is not, open it as a regular file */
-            fio->fp = gmx_ffopen(fn, newmode);
-        }
 
         /* for appending seek to end of file to make sure ftell gives correct position
          * important for checkpointing */
@@ -384,8 +350,6 @@ t_fileio *gmx_fio_open(const char *fn, const char *mode)
     fio->bRead             = bRead;
     fio->bReadWrite        = bReadWrite;
     fio->bDouble           = (sizeof(real) == sizeof(double));
-    fio->bDebug            = FALSE;
-    fio->bOpen             = TRUE;
 
     /* and now insert this file into the list of open files. */
     gmx_fio_insert(fio);
@@ -396,12 +360,7 @@ static int gmx_fio_close_locked(t_fileio *fio)
 {
     int rc = 0;
 
-    if (!fio->bOpen)
-    {
-        gmx_fatal(FARGS, "File %s closed twice!\n", fio->fn);
-    }
-
-    if (ftp_is_xdr(fio->iFTP))
+    if (fio->xdr != NULL)
     {
         xdr_destroy(fio->xdr);
         sfree(fio->xdr);
@@ -412,7 +371,6 @@ static int gmx_fio_close_locked(t_fileio *fio)
         rc = gmx_ffclose(fio->fp); /* fclose returns 0 if happy */
 
     }
-    fio->bOpen = FALSE;
 
     return rc;
 }
@@ -425,10 +383,6 @@ int gmx_fio_close(t_fileio *fio)
     /* We don't want two processes operating on the list at the same time */
     tMPI_Thread_mutex_lock(&open_file_mutex);
 
-    if (fio->iFTP == efTNG)
-    {
-        gmx_incons("gmx_fio_close should not be called on a TNG file");
-    }
     gmx_fio_lock(fio);
     /* first remove it from the list */
     gmx_fio_remove(fio);
@@ -448,7 +402,7 @@ int gmx_fio_fp_close(t_fileio *fio)
 {
     int rc = 0;
     gmx_fio_lock(fio);
-    if (!ftp_is_xdr(fio->iFTP))
+    if (fio->xdr == NULL)
     {
         rc      = gmx_ffclose(fio->fp); /* fclose returns 0 if happy */
         fio->fp = NULL;
@@ -643,7 +597,7 @@ int gmx_fio_get_output_file_positions(gmx_file_position_t **p_outputfiles,
     {
         /* Skip the checkpoint files themselves, since they could be open when
            we call this routine... */
-        if (cur->bOpen && !cur->bRead && cur->iFTP != efCPT)
+        if (!cur->bRead && cur->iFTP != efCPT)
         {
             /* This is an output file currently open for writing, add it */
             if (nfiles == nalloc)
@@ -674,20 +628,6 @@ int gmx_fio_get_output_file_positions(gmx_file_position_t **p_outputfiles,
 }
 
 
-void gmx_fio_setprecision(t_fileio *fio, gmx_bool bDouble)
-{
-    gmx_fio_lock(fio);
-    fio->bDouble = bDouble;
-    gmx_fio_unlock(fio);
-}
-
-void gmx_fio_setdebug(t_fileio *fio, gmx_bool bDebug)
-{
-    gmx_fio_lock(fio);
-    fio->bDebug = bDebug;
-    gmx_fio_unlock(fio);
-}
-
 char *gmx_fio_getname(t_fileio *fio)
 {
     char *ret;
@@ -748,11 +688,6 @@ static int gmx_fio_int_fsync(t_fileio *fio)
     {
         rc = gmx_fsync(fio->fp);
     }
-    else if (fio->xdr) /* this should normally not happen */
-    {
-        rc = gmx_fsync((FILE*) fio->xdr->x_private);
-        /* ^ is this actually OK? */
-    }
 
     return rc;
 }
@@ -779,7 +714,7 @@ t_fileio *gmx_fio_all_output_fsync(void)
     cur = gmx_fio_get_first();
     while (cur)
     {
-        if (cur->bOpen && !cur->bRead)
+        if (!cur->bRead)
         {
             /* if any of them fails, return failure code */
             int rc = gmx_fio_int_fsync(cur);

diff --git a/src/gromacs/fileio/gmxfio.h b/src/gromacs/fileio/gmxfio.h
index dabf39c7eb9878ecb256bc38f9b9b8153bcf2a3f..d08249e4a15ac045784afb177c6f1d46d60cc510 100644 (file)
--- a/src/gromacs/fileio/gmxfio.h
+++ b/src/gromacs/fileio/gmxfio.h
@@ -97,9 +97,6 @@ int gmx_fio_fclose(FILE *fp);
  * Change properties of the open file
  ********************************************************/
 
-void gmx_fio_setprecision(t_fileio *fio, gmx_bool bDouble);
-/* Select the floating point precision for reading and writing files */
-
 char *gmx_fio_getname(t_fileio *fio);
 /* Return the filename corresponding to the fio index */
 
@@ -109,9 +106,6 @@ int gmx_fio_getftp(t_fileio *fio);
     was opened as a specific file type and changing that midway is most
     likely an evil hack. */
 
-void gmx_fio_setdebug(t_fileio *fio, gmx_bool bDebug);
-/* Set the debug mode */
-
 gmx_bool gmx_fio_getread(t_fileio *fio);
 /* Return  whether read mode is on in fio  */
 

diff --git a/src/gromacs/fileio/tpxio.c b/src/gromacs/fileio/tpxio.c
index 63f76d6f1c7a6aaf67a6bf6aa989b23f683f7cb7..509aa4a15d75b25879d458db6b048e87510ade5f 100644 (file)
--- a/src/gromacs/fileio/tpxio.c
+++ b/src/gromacs/fileio/tpxio.c
@@ -3222,8 +3222,6 @@ static void do_tpxheader(t_fileio *fio, gmx_bool bRead, t_tpxheader *tpx,
     int       idum = 0;
     real      rdum = 0;
 
-    gmx_fio_setdebug(fio, bDebugMode());
-
     /* XDR binary topology file */
     precision = sizeof(real);
     if (bRead)