Fix return values of frame-reading functions

This function was based on read_first_x that returned the number of
atoms, and was documented to do the same, but has always returned a
logical boolean about whether a frame has been read. This led to
aspects of gmx spatial and gmx trjcat -demux being broken.

Fixed by returning a proper bool, and fixing the remaining logic that
used the return value in a non-boolean sense.

Refs #2157

Change-Id: Ic871b56f68c7dbc654ab11b34ff82932353e6ceb

diff --git a/src/gromacs/fileio/trxio.cpp b/src/gromacs/fileio/trxio.cpp
index 1786c17ead287f3a217e43757de086487a5b6483..20ce0c01739733294ea578086c28b82242d47ab1 100644 (file)
--- a/src/gromacs/fileio/trxio.cpp
+++ b/src/gromacs/fileio/trxio.cpp
@@ -810,14 +810,14 @@ static int pdb_first_x(t_trxstatus *status, FILE *fp, t_trxframe *fr)
     return fr->natoms;
 }
 
-gmx_bool read_next_frame(const gmx_output_env_t *oenv, t_trxstatus *status, t_trxframe *fr)
+bool read_next_frame(const gmx_output_env_t *oenv, t_trxstatus *status, t_trxframe *fr)
 {
     real     pt;
     int      ct;
-    gmx_bool bOK, bRet, bMissingData = FALSE, bSkip = FALSE;
+    gmx_bool bOK, bMissingData = FALSE, bSkip = FALSE;
+    bool     bRet = false;
     int      ftp;
 
-    bRet = FALSE;
     pt   = status->tf;
 
     do
@@ -908,7 +908,7 @@ gmx_bool read_next_frame(const gmx_output_env_t *oenv, t_trxstatus *status, t_tr
                 }
                 else if (ct > 0)
                 {
-                    bRet = FALSE;
+                    bRet = false;
                 }
                 else
                 {
@@ -933,8 +933,8 @@ gmx_bool read_next_frame(const gmx_output_env_t *oenv, t_trxstatus *status, t_tr
     return bRet;
 }
 
-int read_first_frame(const gmx_output_env_t *oenv, t_trxstatus **status,
-                     const char *fn, t_trxframe *fr, int flags)
+bool read_first_frame(const gmx_output_env_t *oenv, t_trxstatus **status,
+                      const char *fn, t_trxframe *fr, int flags)
 {
     t_fileio      *fio = nullptr;
     gmx_bool       bFirst, bOK;

diff --git a/src/gromacs/fileio/trxio.h b/src/gromacs/fileio/trxio.h
index 0f6495967d518fce1020c2a763285710df8d0ea6..639d22ed1f2b537f0a569c31c2d0d1f63bc4b2f9 100644 (file)
--- a/src/gromacs/fileio/trxio.h
+++ b/src/gromacs/fileio/trxio.h
@@ -198,19 +198,19 @@ int check_times(real t);
 #define DATA_NOT_OK   (1<<1)
 #define FRAME_NOT_OK  (HEADER_NOT_OK | DATA_NOT_OK)
 
-int read_first_frame(const gmx_output_env_t *oenv, t_trxstatus **status,
-                     const char *fn, struct t_trxframe *fr, int flags);
+bool read_first_frame(const gmx_output_env_t *oenv, t_trxstatus **status,
+                      const char *fn, struct t_trxframe *fr, int flags);
 /* Read the first frame which is in accordance with flags, which are
  * defined further up in this file.
  * Memory will be allocated for flagged entries.
  * The flags are copied to fr for subsequent calls to read_next_frame.
- * Returns TRUE when succeeded, FALSE otherwise.
+ * Returns true when succeeded, false otherwise.
  */
 
-gmx_bool read_next_frame(const gmx_output_env_t *oenv, t_trxstatus *status,
-                         struct t_trxframe *fr);
+bool read_next_frame(const gmx_output_env_t *oenv, t_trxstatus *status,
+                     struct t_trxframe *fr);
 /* Reads the next frame which is in accordance with fr->flags.
- * Returns TRUE when succeeded, FALSE otherwise.
+ * Returns true when succeeded, false otherwise.
  */
 
 int read_first_x(const gmx_output_env_t *oenv, t_trxstatus **status,

diff --git a/src/gromacs/gmxana/gmx_trjcat.cpp b/src/gromacs/gmxana/gmx_trjcat.cpp
index 1b41ebb3c5e1c8ff5f02b960ead5ee1cbe53d3b3..6a97e3932b4134fc3719f298f1dd0910d588c8f3 100644 (file)
--- a/src/gromacs/gmxana/gmx_trjcat.cpp
+++ b/src/gromacs/gmxana/gmx_trjcat.cpp
@@ -77,7 +77,7 @@ static void scan_trj_files(char **fnms, int nfiles, real *readtime,
     int          i, natoms = 0;
     t_trxstatus *status;
     t_trxframe   fr;
-    gmx_bool     ok;
+    bool         ok;
 
     for (i = 0; i < nfiles; i++)
     {
@@ -312,7 +312,7 @@ static void do_demux(int nset, char *fnms[], char *fnms_out[], int nval,
                      real **value, real *time, real dt_remd, int isize,
                      int index[], real dt, const gmx_output_env_t *oenv)
 {
-    int           i, j, k, natoms, nnn;
+    int           i, j, k, natoms;
     t_trxstatus **fp_in, **fp_out;
     gmx_bool      bCont, *bSet;
     real          t, first_time = 0;
@@ -325,16 +325,16 @@ static void do_demux(int nset, char *fnms[], char *fnms_out[], int nval,
     t      = -1;
     for (i = 0; (i < nset); i++)
     {
-        nnn = read_first_frame(oenv, &(fp_in[i]), fnms[i], &(trx[i]),
-                               TRX_NEED_X);
+        read_first_frame(oenv, &(fp_in[i]), fnms[i], &(trx[i]),
+                         TRX_NEED_X);
         if (natoms == -1)
         {
-            natoms     = nnn;
+            natoms     = trx[i].natoms;
             first_time = trx[i].time;
         }
-        else if (natoms != nnn)
+        else if (natoms != trx[i].natoms)
         {
-            gmx_fatal(FARGS, "Trajectory file %s has %d atoms while previous trajs had %d atoms", fnms[i], nnn, natoms);
+            gmx_fatal(FARGS, "Trajectory file %s has %d atoms while previous trajs had %d atoms", fnms[i], trx[i].natoms, natoms);
         }
         if (t == -1)
         {