Cleaning up some bad coding practices

- missing return statement
- recommended parenthesis
- uneeded declarations
- statement without effect
- missing switch/break or undocumented fall-through
- unused static variables

Unless I'm mistaken none of the changes has any effect.

Change-Id: I060ce5b50d8e7df3a97917633fc77d7d2e8481a9

diff --git a/include/types/ifunc.h b/include/types/ifunc.h
index 41230b02c3237390e1482074ee3c9a3abc54467e..72026c57c023a1f733e3cef406c89da79438eb2c 100644 (file)
--- a/include/types/ifunc.h
+++ b/include/types/ifunc.h
@@ -105,13 +105,13 @@ typedef struct
 #define NRFP(ftype)  (NRFPA(ftype)+NRFPB(ftype))
 #define NRAL(ftype) (interaction_function[(ftype)].nratoms)
 
-#define IS_CHEMBOND(ftype) (interaction_function[(ftype)].nratoms==2 && interaction_function[(ftype)].flags & IF_CHEMBOND)
+#define IS_CHEMBOND(ftype) (interaction_function[(ftype)].nratoms==2 && (interaction_function[(ftype)].flags & IF_CHEMBOND))
 /* IS_CHEMBOND tells if function type ftype represents a chemical bond */
 
 /* IS_ANGLE tells if a function type ftype represents an angle 
  * Per Larsson, 2007-11-06
  */
-#define IS_ANGLE(ftype) (interaction_function[(ftype)].nratoms==3 && interaction_function[(ftype)].flags & IF_ATYPE)
+#define IS_ANGLE(ftype) (interaction_function[(ftype)].nratoms==3 && (interaction_function[(ftype)].flags & IF_ATYPE))
 #define IS_VSITE(ftype) (interaction_function[(ftype)].flags & IF_VSITE)
 
 #define IS_TABULATED(ftype) (interaction_function[(ftype)].flags & IF_TABULATED)

diff --git a/src/gmxlib/bondfree.c b/src/gmxlib/bondfree.c
index f27df6d4790fb925c4366de6b3c8f1d67894a69f..273804f7010efec7e8c0c56245f693dae4fddbea 100644 (file)
--- a/src/gmxlib/bondfree.c
+++ b/src/gmxlib/bondfree.c
@@ -2611,7 +2611,7 @@ void calc_bonds(FILE *fplog,const gmx_multisim_t *ms,
   /* Loop over all bonded force types to calculate the bonded forces */
   for(ftype=0; (ftype<F_NRE); ftype++) {
          if(ftype<F_GB12 || ftype>F_GB14) {
-    if (interaction_function[ftype].flags & IF_BOND &&
+    if ((interaction_function[ftype].flags & IF_BOND) &&
        !(ftype == F_CONNBONDS || ftype == F_POSRES)) {
       nbonds=idef->il[ftype].nr;
       if (nbonds > 0) {
@@ -2699,7 +2699,7 @@ void calc_bonds_lambda(FILE *fplog,
   for(ftype=0; (ftype<F_NRE); ftype++) {
       if(ftype<F_GB12 || ftype>F_GB14) {
           
-          if (interaction_function[ftype].flags & IF_BOND &&
+          if ((interaction_function[ftype].flags & IF_BOND) &&
               !(ftype == F_CONNBONDS || ftype == F_POSRES)) 
           {
               nbonds_np = idef->il[ftype].nr_nonperturbed;

diff --git a/src/gmxlib/checkpoint.c b/src/gmxlib/checkpoint.c
index b29034741ff81fe655ad6c035a722f1c0b0f8930..41a6a517531ceba57f036ddbfaba2c45b0794464 100644 (file)
--- a/src/gmxlib/checkpoint.c
+++ b/src/gmxlib/checkpoint.c
@@ -1666,7 +1666,7 @@ static void read_checkpoint(const char *fn,FILE **pfplog,
         cp_error();
     }
     *bReadEkin = ((flags_eks & (1<<eeksEKINH)) || (flags_eks & (1<<eeksEKINF)) || (flags_eks & (1<<eeksEKINO)) ||
-                  (flags_eks & (1<<eeksEKINSCALEF)) | (flags_eks & (1<<eeksEKINSCALEH)) | (flags_eks & (1<<eeksVSCALE)));
+                  ((flags_eks & (1<<eeksEKINSCALEF)) | (flags_eks & (1<<eeksEKINSCALEH)) | (flags_eks & (1<<eeksVSCALE))));
     
     ret = do_cpt_enerhist(gmx_fio_getxdr(fp),TRUE,
                           flags_enh,&state->enerhist,NULL);

diff --git a/src/gmxlib/selection/sm_insolidangle.c b/src/gmxlib/selection/sm_insolidangle.c
index 1f72fcf1815b40d5a71d681dd5fc80c65042d333..9ed4aaad3e3967e849add22163430483627c484e 100644 (file)
--- a/src/gmxlib/selection/sm_insolidangle.c
+++ b/src/gmxlib/selection/sm_insolidangle.c
@@ -208,9 +208,6 @@ init_data_insolidangle(int npar, gmx_ana_selparam_t *param);
 /** Initializes the \p insolidangle selection method. */
 static int
 init_insolidangle(t_topology *top, int npar, gmx_ana_selparam_t *param, void *data);
-/** Sets the COM/COG data for the \p insolidangle selection method. */
-static void
-set_comg_insolidangle(gmx_ana_pos_t *pos, void *data);
 /** Frees the data allocated for the \p insolidangle selection method. */
 static void
 free_data_insolidangle(void *data);

diff --git a/src/gmxlib/tcontrol.c b/src/gmxlib/tcontrol.c
index 3c609b90a7966f7ed26cc874cf6906dbd8e9fcbc..f4c2a39e30761ac1ce18b77d965b9f3fd350da65 100644 (file)
--- a/src/gmxlib/tcontrol.c
+++ b/src/gmxlib/tcontrol.c
@@ -63,24 +63,6 @@ static t_timecontrol timecontrol[TNR] = {
 static tMPI_Thread_mutex_t tc_mutex=TMPI_THREAD_MUTEX_INITIALIZER;
 #endif
 
-typedef struct {
-  real tfactor;
-  const char *tstr,*xvgstr;
-} t_timeconvert;
-
-static const t_timeconvert timeconvert[] = {
-    { 0,                   NULL,  NULL       },
-    { 1e3,                "fs",  "fs"       },
-    { 1,                  "ps",  "ps"       },
-    { 1e-3,               "ns",  "ns"       },
-    { 1e-6,               "us",  "\\mus"    }, 
-    { 1e-9,               "ms",  "ms"       },
-    { 1e-12,              "s",   "s"        },
-    { (1.0/60.0)*1e-12,    "m",   "m"        },
-    { (1.0/3600.0)*1e-12,  "h",   "h"        },
-    { 0,                  NULL,  NULL       }
-};
-
 gmx_bool bTimeSet(int tcontrol)
 {
     gmx_bool ret;

diff --git a/src/gmxlib/trxio.c b/src/gmxlib/trxio.c
index 9874339d3e61adfcccf238f1721ea29573d11b68..a4dadff37b461c6447120b3710afa677857bcca3 100644 (file)
--- a/src/gmxlib/trxio.c
+++ b/src/gmxlib/trxio.c
@@ -222,6 +222,7 @@ int write_trxframe_indexed(t_trxstatus *status,t_trxframe *fr,int nind,
       for(i=0; i<nind; i++) 
        copy_rvec(fr->f[ind[i]],fout[i]);
     }
+  /* no break */
   case efXTC:
   case efG87:
     if (fr->bX) {
@@ -277,6 +278,7 @@ int write_trxframe_indexed(t_trxstatus *status,t_trxframe *fr,int nind,
   case efTRR:
     if (vout) sfree(vout);
     if (fout) sfree(fout);
+  /* no break */
   case efXTC:
   case efG87:
     sfree(xout);
@@ -747,13 +749,13 @@ gmx_bool read_next_frame(const output_env_t oenv,t_trxstatus *status,t_trxframe
     }
     
     if (bRet) {
-      bMissingData = ((fr->flags & TRX_NEED_X && !fr->bX) ||
-                     (fr->flags & TRX_NEED_V && !fr->bV) ||
-                     (fr->flags & TRX_NEED_F && !fr->bF));
+      bMissingData = (((fr->flags & TRX_NEED_X) && !fr->bX) ||
+                     ((fr->flags & TRX_NEED_V) && !fr->bV) ||
+                     ((fr->flags & TRX_NEED_F) && !fr->bF));
       bSkip = FALSE;
       if (!bMissingData) {
        ct=check_times2(fr->time,fr->t0,fr->tpf,fr->tppf,fr->bDouble);
-       if (ct == 0 || (fr->flags & TRX_DONT_SKIP && ct<0)) {
+       if (ct == 0 || ((fr->flags & TRX_DONT_SKIP) && ct<0)) {
          printcount(status, oenv,fr->time,FALSE);
        } else if (ct > 0)
          bRet = FALSE;

diff --git a/src/gmxlib/wman.c b/src/gmxlib/wman.c
index 4b42bb11ee87bd2eb6c5b289b6753522f68d3de4..4c51f10a80cdea832bc640db35d9bccac44394be 100644 (file)
--- a/src/gmxlib/wman.c
+++ b/src/gmxlib/wman.c
@@ -1025,6 +1025,7 @@ static void write_py(FILE *out,const char *program,
       for(j=2; (pa[i].u.c[j] != NULL); j++)
        fprintf(out,",'%s'",pa[i].u.c[j]);
       fprintf(out,"],%d))\n",is_hidden(&(pa[i])));
+      break;
     default:
       break;
     }

diff --git a/src/kernel/topdirs.c b/src/kernel/topdirs.c
index b5c3c0a1fb37e84ffa9275041ed26c4fefa6285a..14dd59fc00a7388bf45795a81912836e442d6509 100644 (file)
--- a/src/kernel/topdirs.c
+++ b/src/kernel/topdirs.c
@@ -76,6 +76,7 @@ int ifunc_index(directive d,int type)
       gmx_fatal(FARGS,"Invalid bond type %d",type);
       break;
     }
+    break;
   case d_angles:
   case d_angletypes:
     switch (type) {
@@ -97,7 +98,8 @@ int ifunc_index(directive d,int type)
       gmx_fatal(FARGS,"Invalid angle type %d",type);
       break;
     }
-    case d_pairs:
+    break;
+  case d_pairs:
   case d_pairtypes:
     if (type == 1 || (d == d_pairtypes && type == 2))
       return F_LJ14;
@@ -105,6 +107,7 @@ int ifunc_index(directive d,int type)
       return F_LJC14_Q;
     else
       gmx_fatal(FARGS,"Invalid pairs type %d",type);
+    break;
   case d_pairs_nb:
     return F_LJC_PAIRS_NB;
   case d_dihedrals:
@@ -152,6 +155,7 @@ int ifunc_index(directive d,int type)
     default:
       gmx_fatal(FARGS,"Invalid vsites3 type %d",type);
     }
+    break;
   case d_vsites4:
       switch (type) {
           case 1:
@@ -161,6 +165,7 @@ int ifunc_index(directive d,int type)
           default:
               gmx_fatal(FARGS,"Invalid vsites4 type %d",type);
       }
+      break;
   case d_vsitesn:
     return F_VSITEN; 
   case d_constraints:
@@ -173,6 +178,7 @@ int ifunc_index(directive d,int type)
     default:
       gmx_fatal(FARGS,"Invalid constraints type %d",type);
     }
+    break;
   case d_settles:
     return F_SETTLE;
   case d_position_restraints:
@@ -181,9 +187,11 @@ int ifunc_index(directive d,int type)
       return F_POSRES;
     case 2:
       gmx_fatal(FARGS,"Water polarization should now be listed under [ water_polarization ]\n");
+      break;
     default:
       gmx_fatal(FARGS,"Invalid position restraint type %d",type);
     }
+    break;
   case d_polarization:
     return F_POLARIZATION;
   case d_thole_polarization:

diff --git a/src/mdlib/domdec.c b/src/mdlib/domdec.c
index 2bae6564136acc54860a52c3e5bcae0ca01241ad..ecf3ac3c95f08ebed2402249fc634935785f71dc 100644 (file)
--- a/src/mdlib/domdec.c
+++ b/src/mdlib/domdec.c
@@ -1481,7 +1481,7 @@ void dd_collect_state(gmx_domdec_t *dd,
     }
     for(est=0; est<estNR; est++)
     {
-        if (EST_DISTR(est) && state_local->flags & (1<<est))
+        if (EST_DISTR(est) && (state_local->flags & (1<<est)))
         {
             switch (est) {
             case estX:
@@ -1563,7 +1563,7 @@ static void dd_realloc_state(t_state *state,rvec **f,int nalloc)
     
     for(est=0; est<estNR; est++)
     {
-        if (EST_DISTR(est) && state->flags & (1<<est))
+        if (EST_DISTR(est) && (state->flags & (1<<est)))
         {
             switch(est) {
             case estX:
@@ -1756,7 +1756,7 @@ static void dd_distribute_state(gmx_domdec_t *dd,t_block *cgs,
     }
     for(i=0; i<estNR; i++)
     {
-        if (EST_DISTR(i) && state_local->flags & (1<<i))
+        if (EST_DISTR(i) && (state_local->flags & (1<<i)))
         {
             switch (i) {
             case estX:
@@ -4132,7 +4132,7 @@ static void rotate_state_atom(t_state *state,int a)
 
     for(est=0; est<estNR; est++)
     {
-        if (EST_DISTR(est) && state->flags & (1<<est)) {
+        if (EST_DISTR(est) && (state->flags & (1<<est))) {
             switch (est) {
             case estX:
                 /* Rotate the complete state; for a rectangular box only */
@@ -7993,7 +7993,7 @@ static void dd_sort_state(gmx_domdec_t *dd,int ePBC,
     /* Reorder the state */
     for(i=0; i<estNR; i++)
     {
-        if (EST_DISTR(i) && state->flags & (1<<i))
+        if (EST_DISTR(i) && (state->flags & (1<<i)))
         {
             switch (i)
             {

diff --git a/src/mdlib/edsam.c b/src/mdlib/edsam.c
index f4e770aca4343c26682ff277af8ec0b17194c167..8359863d3b2081edc924c8c267fe3b0b61a3ba36 100644 (file)
--- a/src/mdlib/edsam.c
+++ b/src/mdlib/edsam.c
@@ -1497,9 +1497,7 @@ static int read_edi(FILE* in, gmx_edsam_t ed,t_edpar *edi,int nr_mdatoms, int ed
     {
         if (readmagic==666 || readmagic==667 || readmagic==668)
             gmx_fatal(FARGS,"Wrong magic number: Use newest version of make_edi to produce edi file");
-        else if (readmagic == 669)
-            ;
-        else
+        else if (readmagic != 669)
             gmx_fatal(FARGS,"Wrong magic number %d in %s",readmagic,ed->edinam);
     }
 

diff --git a/src/mdlib/fft5d.c b/src/mdlib/fft5d.c
index dbbcdaf4e053e177836f8c1b45735abaf2909f95..e2f5266ec423b9eeb791f11034034274a3031be8 100644 (file)
--- a/src/mdlib/fft5d.c
+++ b/src/mdlib/fft5d.c
@@ -101,6 +101,7 @@ static int lfactor(int z) {
        int i;
        for (i=sqrt(z);;i--)
                if (z%i==0) return i;
+       return 1;
 }
 
 /* largest factor */
@@ -109,6 +110,7 @@ static int l2factor(int z) {
        if (z==1) return 1;
        for (i=z/2;;i--)
                if (z%i==0) return i;
+       return 1;
 }
 
 /* largest prime factor: WARNING: slow recursion, only use for small numbers */
@@ -795,7 +797,7 @@ static void compute_offsets(fft5d_plan plan, int xs[], int xl[], int xc[], int N
             rotate(NG);            
         }
     }
-    if (plan->flags&FFT5D_REALCOMPLEX && ((!(plan->flags&FFT5D_BACKWARD) && s==0) || (plan->flags&FFT5D_BACKWARD && s==2))) {
+    if ((plan->flags&FFT5D_REALCOMPLEX) && ((!(plan->flags&FFT5D_BACKWARD) && s==0) || ((plan->flags&FFT5D_BACKWARD) && s==2))) {
         xl[0] = rC[s];
     }
 }
@@ -1033,7 +1035,7 @@ void fft5d_compare_data(const t_complex* lin, const t_complex* in, fft5d_plan pl
     int x,y,z,l;
     int *coor = plan->coor;
     int ll=2; /*compare ll values per element (has to be 2 for complex)*/
-    if (plan->flags&FFT5D_REALCOMPLEX && plan->flags&FFT5D_BACKWARD) 
+    if ((plan->flags&FFT5D_REALCOMPLEX) && (plan->flags&FFT5D_BACKWARD))
     {
         ll=1;
     }

diff --git a/src/mdlib/wall.c b/src/mdlib/wall.c
index 168d56a532d677e23800db06d6e673fc5024ec71..beb4d7cf5c34483e11bb6c93af59c9c12cef3c9d 100644 (file)
--- a/src/mdlib/wall.c
+++ b/src/mdlib/wall.c
@@ -169,7 +169,7 @@ real do_walls(t_inputrec *ir,t_forcerec *fr,matrix box,t_mdatoms *md,
                 at = type[i];
                 Cd = nbfp[ntw[w]+2*at];
                 Cr = nbfp[ntw[w]+2*at+1];
-                if (!((Cd==0 && Cr==0) || egp_flags[ggid] & EGP_EXCL))
+                if (!((Cd==0 && Cr==0) || (egp_flags[ggid] & EGP_EXCL)))
                 {
                     if (w == 0)
                     {

diff --git a/src/ngmx/nener.c b/src/ngmx/nener.c
index 42b7d0d306e3fb6382ca289dfcfa006b0bda53f9..b21c72695b4a4337c7f0830038323520b54f45e3 100644 (file)
--- a/src/ngmx/nener.c
+++ b/src/ngmx/nener.c
@@ -125,7 +125,7 @@ static gmx_bool EWCallBack(t_x11 *x11,XEvent *event, Window w, void *data)
   case ClientMessage:
     ew->etype=event->xclient.data.l[0];
     ExposeWin(x11->disp,ew->wd.self);
-    /* Fall thru... */
+    /* no break */
   case ButtonRelease:
     hide_menu(x11,ew->selener);
     break;

diff --git a/src/ngmx/ngmx.c b/src/ngmx/ngmx.c
index 5c399573129135acffbd65f7f6b4a1d8e04d107f..a3ce8e14b9843b587b2c935c4594fd3129ac4631 100644 (file)
--- a/src/ngmx/ngmx.c
+++ b/src/ngmx/ngmx.c
@@ -60,8 +60,6 @@
 void init_gmx(t_x11 *x11,char *program,int nfile,t_filenm fnm[],
               const output_env_t oenv);
 
-int EventSignaller(t_manager *man);
-
 static void dump_xw(char *dispname,Window w,char *fn)
 {
   char comm[256];

diff --git a/src/tools/acf.test.c b/src/tools/acf.test.c
index 6bde6ce9ee54a382b5f3d4618423c00b606c760c..5babfe2aabd60bf05f3d09c383762511b38440b5 100644 (file)
--- a/src/tools/acf.test.c
+++ b/src/tools/acf.test.c
@@ -106,9 +106,10 @@ int main(int argc,char *argv[])
       break;
     case 5:
       if (i==0)
-       data[i] = 1;
+        data[i] = 1;
       else
-       data[i] = sin(x)/(x);
+        data[i] = sin(x)/(x);
+      break;
     default:
       /* Data remains 0.0 */
       break;

diff --git a/src/tools/expfit.c b/src/tools/expfit.c
index 7ea7566b18809394dee4edda6d5cc287524e4e31..c16144ddb9903c47027d9b1ba58d20378c2ea443 100644 (file)
--- a/src/tools/expfit.c
+++ b/src/tools/expfit.c
@@ -69,12 +69,6 @@ const char *longs_ffn[effnNR] = {
   "y = a2*ee(a1,x) + (1-a2)*ee(a2,x)"
 };
 
-extern gmx_bool mrqmin(real x[],real y[],real sig[],int ndata,real a[],
-                  int ma,int lista[],int mfit,real **covar,real **alpha,
-                  real *chisq,
-                  void (*funcs)(real x,real a[],real *y,real dyda[]),
-                  real *alamda);
-
 extern gmx_bool mrqmin_new(real x[],real y[],real sig[],int ndata,real a[], 
                       int ia[],int ma,real **covar,real **alpha,real *chisq, 
                       void (*funcs)(real, real [], real *, real []), 

diff --git a/src/tools/gmx_editconf.c b/src/tools/gmx_editconf.c
index 3662585b71abc74a999c76e3d5e2581d1f37abb1..7d33eb4a251442bd3068a21697e0e9a4d0363a2b 100644 (file)
--- a/src/tools/gmx_editconf.c
+++ b/src/tools/gmx_editconf.c
@@ -79,8 +79,6 @@ typedef struct
     int nsatm;
     t_simat sat[3];
 } t_simlist;
-static const char *pdbtp[epdbNR] =
-    { "ATOM  ", "HETATM" };
 
 real calc_mass(t_atoms *atoms, gmx_bool bGetMass, gmx_atomprop_t aps)
 {

diff --git a/src/tools/gmx_enemat.c b/src/tools/gmx_enemat.c
index 480e4ded1c97073aae828c0b8efde4bc2b11c8e4..48b6bd7497c0ba3748e3bedd16ab34e6a81fd404 100644 (file)
--- a/src/tools/gmx_enemat.c
+++ b/src/tools/gmx_enemat.c
@@ -352,8 +352,6 @@ int gmx_enemat(int argc,char *argv[])
     }
    
     emid = 0.0;/*(emin+emax)*0.5;*/
-    for(m=0; (m<egNR); m++)
-      egrp_nm[m]=egrp_nm[m];
     egrp_nm[egTotal]="total";
     for (m=0; (m<egNR+egSP); m++) 
       if (egrp_use[m]) {

diff --git a/src/tools/make_edi.c b/src/tools/make_edi.c
index 409fb4764c9d9819e7dae4addb8aac640b68d481..22694d0f2a900e912ce717243306a97d613ced3e 100644 (file)
--- a/src/tools/make_edi.c
+++ b/src/tools/make_edi.c
@@ -235,14 +235,15 @@ int sscan_list(int *list[], const char *str, const char *listname) {
 
        /* format error occured */
        case sError:
-       gmx_fatal(FARGS,"Error in the list of eigenvectors for %s at pos %d with char %c",listname,pos-startpos,*(pos-1));
-
+          gmx_fatal(FARGS,"Error in the list of eigenvectors for %s at pos %d with char %c",listname,pos-startpos,*(pos-1));
+          break;
        /* logical error occured */
        case sZero:
-               gmx_fatal(FARGS,"Error in the list of eigenvectors for %s at pos %d: eigenvector 0 is not valid",listname,pos-startpos);
+                  gmx_fatal(FARGS,"Error in the list of eigenvectors for %s at pos %d: eigenvector 0 is not valid",listname,pos-startpos);
+                  break;
        case sSmaller:
-               gmx_fatal(FARGS,"Error in the list of eigenvectors for %s at pos %d: second index %d is not bigger than %d",listname,pos-startpos,end_number,number);
-
+                  gmx_fatal(FARGS,"Error in the list of eigenvectors for %s at pos %d: second index %d is not bigger than %d",listname,pos-startpos,end_number,number);
+                  break;
      }
    ++pos; /* read next character */
    } /*scanner has finished */

diff --git a/src/tools/mk_angndx.c b/src/tools/mk_angndx.c
index d4b3cfda8bfa4c9c07c8c2d4384188b050b7119d..25215fdcefea4656615449f417efc4e145392edd 100644 (file)
--- a/src/tools/mk_angndx.c
+++ b/src/tools/mk_angndx.c
@@ -173,7 +173,7 @@ static int *select_ftype(const char *opt,int *nft,int *mult)
   if (opt[0] == 'a') {
     *mult = 3;
     for(ftype=0; ftype<F_NRE; ftype++) {
-      if (interaction_function[ftype].flags & IF_ATYPE ||
+      if ((interaction_function[ftype].flags & IF_ATYPE) ||
          ftype == F_TABANGLES) {
        (*nft)++;
        srenew(ft,*nft);