Errors take precedence over EOF; use file_error() after operations that

return an "EOF or error" indication - an EOF without an error will
return 0.

In iseries_seek_next_packet(), return an error code of WTAP_ERR_BAD_FILE
and an appropriate error message if we don't find a packet header within
the next ISERIES_MAX_TRACE_LEN lines, don't just return -1 and leave the
error information unchanged.

Setting an argument variable before returning has no effect, so don't do
it (so that we don't leave the mistaken impression that it *is* doing
something).

Clean up indentation.

svn path=/trunk/; revision=46819

8 files changed, 166 insertions, 229 deletions

diff --git a/wiretap/cosine.c b/wiretap/cosine.c
index f0d67306c7..183ca241a7 100644
--- a/wiretap/cosine.c
+++ b/wiretap/cosine.c
@@ -200,8 +200,8 @@ static gboolean empty_line(const gchar *line)
 
 /* Seeks to the beginning of the next packet, and returns the
    byte offset. Copy the header line to hdr. Returns -1 on failure,
-   and sets "*err" to the error, sets "*err_info" to null or an
-   additional error string, and sets hdr to NULL. */
+   and sets "*err" to the error and sets "*err_info" to null or an
+   additional error string. */
 static gint64 cosine_seek_next_packet(wtap *wth, int *err, gchar **err_info,
 	char *hdr)
 {
@@ -213,27 +213,18 @@ static gint64 cosine_seek_next_packet(wtap *wth, int *err, gchar **err_info,
 		if (cur_off == -1) {
 			/* Error */
 			*err = file_error(wth->fh, err_info);
-			hdr = NULL;
 			return -1;
 		}
-		if (file_gets(buf, sizeof(buf), wth->fh) != NULL) {
-			if (strstr(buf, COSINE_REC_MAGIC_STR1) ||
-			    strstr(buf, COSINE_REC_MAGIC_STR2)) {
-				g_strlcpy(hdr, buf, COSINE_LINE_LENGTH);
-				return cur_off;
-			}
-		} else {
-			if (file_eof(wth->fh)) {
-				/* We got an EOF. */
-				*err = 0;
-			} else {
-				/* We got an error. */
-				*err = file_error(wth->fh, err_info);
-			}
-			break;
+		if (file_gets(buf, sizeof(buf), wth->fh) == NULL) {
+			*err = file_error(wth->fh, err_info);
+			return -1;
+		}
+		if (strstr(buf, COSINE_REC_MAGIC_STR1) ||
+		    strstr(buf, COSINE_REC_MAGIC_STR2)) {
+			g_strlcpy(hdr, buf, COSINE_LINE_LENGTH);
+			return cur_off;
 		}
 	}
-	hdr = NULL;
 	return -1;
 }
 
diff --git a/wiretap/dbs-etherwatch.c b/wiretap/dbs-etherwatch.c
index 5a46686326..ba0c1a615d 100644
--- a/wiretap/dbs-etherwatch.c
+++ b/wiretap/dbs-etherwatch.c
@@ -124,13 +124,8 @@ static gint64 dbs_etherwatch_seek_next_packet(wtap *wth, int *err,
       level = 0;
     }
   }
-  if (file_eof(wth->fh)) {
-    /* We got an EOF. */
-    *err = 0;
-  } else {
-    /* We got an error. */
-    *err = file_error(wth->fh, err_info);
-  }
+  /* EOF or error. */
+  *err = file_error(wth->fh, err_info);
   return -1;
 }
 
@@ -155,33 +150,28 @@ static gboolean dbs_etherwatch_check_file_type(wtap *wth, int *err,
 	buf[DBS_ETHERWATCH_LINE_LENGTH-1] = 0;
 
 	for (line = 0; line < DBS_ETHERWATCH_HEADER_LINES_TO_CHECK; line++) {
-		if (file_gets(buf, DBS_ETHERWATCH_LINE_LENGTH, wth->fh)!=NULL){
-
-			reclen = strlen(buf);
-			if (reclen < DBS_ETHERWATCH_HDR_MAGIC_SIZE)
-				continue;
-
-			level = 0;
-			for (i = 0; i < reclen; i++) {
-				byte = buf[i];
-				if (byte == dbs_etherwatch_hdr_magic[level]) {
-					level++;
-					if (level >=
-					      DBS_ETHERWATCH_HDR_MAGIC_SIZE) {
-						return TRUE;
-					}
+		if (file_gets(buf, DBS_ETHERWATCH_LINE_LENGTH, wth->fh) == NULL) {
+			/* EOF or error. */
+			*err = file_error(wth->fh, err_info);
+			return FALSE;
+		}
+
+		reclen = strlen(buf);
+		if (reclen < DBS_ETHERWATCH_HDR_MAGIC_SIZE)
+			continue;
+
+		level = 0;
+		for (i = 0; i < reclen; i++) {
+			byte = buf[i];
+			if (byte == dbs_etherwatch_hdr_magic[level]) {
+				level++;
+				if (level >=
+				      DBS_ETHERWATCH_HDR_MAGIC_SIZE) {
+					return TRUE;
 				}
-				else
-					level = 0;
 			}
-		}
-		else {
-			/* EOF or error. */
-			if (file_eof(wth->fh))
-				*err = 0;
 			else
-				*err = file_error(wth->fh, err_info);
-			return FALSE;
+				level = 0;
 		}
 	}
 	*err = 0;
diff --git a/wiretap/eyesdn.c b/wiretap/eyesdn.c
index fc25ad344b..c32f5b86bf 100644
--- a/wiretap/eyesdn.c
+++ b/wiretap/eyesdn.c
@@ -121,13 +121,8 @@ static gint64 eyesdn_seek_next_packet(wtap *wth, int *err, gchar **err_info)
 			return cur_off;
 		}
 	}
-	if (file_eof(wth->fh)) {
-		/* We got an EOF. */
-		*err = 0;
-	} else {
-		/* We got an error. */
-		*err = file_error(wth->fh, err_info);
-	}
+	/* EOF or error. */
+	*err = file_error(wth->fh, err_info);
 	return -1;
 }
 
diff --git a/wiretap/iseries.c b/wiretap/iseries.c
index 4603647540..b598be78bd 100644
--- a/wiretap/iseries.c
+++ b/wiretap/iseries.c
@@ -420,58 +420,54 @@ iseries_seek_next_packet (wtap * wth, int *err, gchar **err_info)
    */
   for (line = 0; line < ISERIES_MAX_TRACE_LEN; line++)
     {
-      if (file_gets (buf, ISERIES_LINE_LENGTH, wth->fh) != NULL)
-        {
-          /* Convert UNICODE to ASCII if required and determine    */
-          /* the number of bytes to rewind to beginning of record. */
-          if (iseries->format == ISERIES_FORMAT_UNICODE)
-            {
-              /* buflen is #bytes to 1st 0x0A */
-             buflen = iseries_UNICODE_to_ASCII ((guint8 *) buf, ISERIES_LINE_LENGTH);
-            }
-          else
-            {
-              /* Else buflen is just length of the ASCII string */
-              buflen = (long) strlen (buf);
-            }
-          ascii_strup_inplace (buf);
-          /* If packet header found return the offset */
-          num_items_scanned =
-            sscanf (buf+78,
-                    "%*[ \n\t]ETHV2%*[ .:\n\t]TYPE%*[ .:\n\t]%4s",type);
-          if (num_items_scanned == 1)
-            {
-              /* Rewind to beginning of line */
-              cur_off = file_tell (wth->fh);
-              if (cur_off == -1)
-                {
-                  *err = file_error (wth->fh, err_info);
-                  return -1;
-                }
-              if (file_seek (wth->fh, cur_off - buflen, SEEK_SET, err) == -1)
-                {
-                  return -1;
-                }
-              return cur_off - buflen;
-            }
-        }
-      /* Otherwise we got an error or reached EOF */
-      else
+      if (file_gets (buf, ISERIES_LINE_LENGTH, wth->fh) == NULL)
         {
-          if (file_eof (wth->fh))
-            {
-              /* We got an EOF. */
-              *err = 0;
-            }
-          else
+          /* EOF or error. */
+          *err = file_error (wth->fh, err_info);
+          if (*err != 0)
             {
-              /* We got an error. */
-              *err = file_error (wth->fh, err_info);
+              return -1;
             }
-          return -1;
+          return 0;
         }
+        /* Convert UNICODE to ASCII if required and determine    */
+        /* the number of bytes to rewind to beginning of record. */
+        if (iseries->format == ISERIES_FORMAT_UNICODE)
+          {
+            /* buflen is #bytes to 1st 0x0A */
+           buflen = iseries_UNICODE_to_ASCII ((guint8 *) buf, ISERIES_LINE_LENGTH);
+          }
+        else
+          {
+            /* Else buflen is just length of the ASCII string */
+            buflen = (long) strlen (buf);
+          }
+        ascii_strup_inplace (buf);
+        /* If packet header found return the offset */
+        num_items_scanned =
+          sscanf (buf+78,
+                  "%*[ \n\t]ETHV2%*[ .:\n\t]TYPE%*[ .:\n\t]%4s",type);
+        if (num_items_scanned == 1)
+          {
+            /* Rewind to beginning of line */
+            cur_off = file_tell (wth->fh);
+            if (cur_off == -1)
+              {
+                *err = file_error (wth->fh, err_info);
+                return -1;
+              }
+            if (file_seek (wth->fh, cur_off - buflen, SEEK_SET, err) == -1)
+              {
+                return -1;
+              }
+            return cur_off - buflen;
+          }
     }
 
+  *err = WTAP_ERR_BAD_FILE;
+  *err_info =
+    g_strdup_printf ("iseries: next packet header not found within %d lines",
+             ISERIES_MAX_TRACE_LEN);
   return -1;
 }
 
@@ -620,11 +616,11 @@ iseries_parse_packet (wtap * wth, FILE_T fh,
       if (file_gets (data, ISERIES_LINE_LENGTH, fh) == NULL)
         {
           *err = file_error (fh, err_info);
-          if (*err == 0)
+          if (*err != 0)
             {
-              *err = WTAP_ERR_SHORT_READ;
+              return -1;
             }
-          return -1;
+          return 0;
         }
       /* Convert UNICODE data to ASCII */
       if (iseries->format == ISERIES_FORMAT_UNICODE)
@@ -724,7 +720,7 @@ iseries_parse_packet (wtap * wth, FILE_T fh,
   ascii_offset = 14*2; /* 14-byte Ethernet header, 2 characters per byte */
 
   /*
-   * Start Reading packet contents
+   * Start reading packet contents
    */
   isCurrentPacket = TRUE;
 
@@ -736,19 +732,13 @@ iseries_parse_packet (wtap * wth, FILE_T fh,
       /* Read the next line */
       if (file_gets (data, ISERIES_LINE_LENGTH, fh) == NULL)
         {
-          if (file_eof (fh))
+          *err = file_error (fh, err_info);
+          if (*err == 0)
             {
+              /* Hit the EOF without an error */
               break;
             }
-          else
-            {
-              *err = file_error (fh, err_info);
-              if (*err == 0)
-                {
-                  *err = WTAP_ERR_SHORT_READ;
-                }
-              goto errxit;
-            }
+          goto errxit;
         }
 
       /* Convert UNICODE data to ASCII and determine line length */
diff --git a/wiretap/netscreen.c b/wiretap/netscreen.c
index 8609afa7fa..de878dd64b 100644
--- a/wiretap/netscreen.c
+++ b/wiretap/netscreen.c
@@ -110,8 +110,8 @@ static gboolean info_line(const gchar *line)
 
 /* Seeks to the beginning of the next packet, and returns the
    byte offset. Copy the header line to hdr. Returns -1 on failure,
-   and sets "*err" to the error, sets "*err_info" to null or an
-   additional error string, and sets hdr to NULL. */
+   and sets "*err" to the error and sets "*err_info" to null or an
+   additional error string. */
 static gint64 netscreen_seek_next_packet(wtap *wth, int *err, gchar **err_info,
     char *hdr)
 {
@@ -123,27 +123,19 @@ static gint64 netscreen_seek_next_packet(wtap *wth, int *err, gchar **err_info,
 		if (cur_off == -1) {
 			/* Error */
 			*err = file_error(wth->fh, err_info);
-			hdr = NULL;
 			return -1;
 		}
-		if (file_gets(buf, sizeof(buf), wth->fh) != NULL) {
-			if (strstr(buf, NETSCREEN_REC_MAGIC_STR1) ||
-			    strstr(buf, NETSCREEN_REC_MAGIC_STR2)) {
-				g_strlcpy(hdr, buf, NETSCREEN_LINE_LENGTH);
-				return cur_off;
-			}
-		} else {
-			if (file_eof(wth->fh)) {
-				/* We got an EOF. */
-				*err = 0;
-			} else {
-				/* We got an error. */
-				*err = file_error(wth->fh, err_info);
-			}
+		if (file_gets(buf, sizeof(buf), wth->fh) == NULL) {
+			/* EOF or error. */
+			*err = file_error(wth->fh, err_info);
 			break;
 		}
+		if (strstr(buf, NETSCREEN_REC_MAGIC_STR1) ||
+		    strstr(buf, NETSCREEN_REC_MAGIC_STR2)) {
+			g_strlcpy(hdr, buf, NETSCREEN_LINE_LENGTH);
+			return cur_off;
+		}
 	}
-	hdr = NULL;
 	return -1;
 }
 
diff --git a/wiretap/peektagged.c b/wiretap/peektagged.c
index 98a3cafacb..9792ffd7b7 100644
--- a/wiretap/peektagged.c
+++ b/wiretap/peektagged.c
@@ -108,13 +108,12 @@ static int wtap_file_read_pattern (wtap *wth, const char *pattern, int *err,
     while (*cp)
     {
 	c = file_getc(wth->fh);
-	if (c == EOF) {
-	    if (file_eof(wth->fh))
-		return 0;	/* EOF */
-	    else {
-		*err = file_error(wth->fh, err_info);
+	if (c == EOF)
+	{
+	    *err = file_error(wth->fh, err_info);
+	    if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
 		return -1;	/* error */
-	    }
+	    return 0;	/* EOF */
 	}
 	if (c == *cp)
 	    cp++;
@@ -141,13 +140,12 @@ static int wtap_file_read_till_separator (wtap *wth, char *buffer, int buflen,
     for (cp = buffer, i = 0; i < buflen; i++, cp++)
     {
 	c = file_getc(wth->fh);
-	if (c == EOF) {
-	    if (file_eof(wth->fh))
-		return 0;	/* EOF */
-	    else {
-		*err = file_error(wth->fh, err_info);
+	if (c == EOF)
+	{
+	    *err = file_error(wth->fh, err_info);
+	    if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
 		return -1;	/* error */
-	    }
+	    return 0;	/* EOF */
 	}
 	if (strchr (separators, c) != NULL)
 	{
diff --git a/wiretap/toshiba.c b/wiretap/toshiba.c
index dea3d7c6d6..07f00eb373 100644
--- a/wiretap/toshiba.c
+++ b/wiretap/toshiba.c
@@ -145,13 +145,8 @@ static gint64 toshiba_seek_next_packet(wtap *wth, int *err, gchar **err_info)
       level = 0;
     }
   }
-  if (file_eof(wth->fh)) {
-    /* We got an EOF. */
-    *err = 0;
-  } else {
-    /* We got an error. */
-    *err = file_error(wth->fh, err_info);
-  }
+  /* EOF or error. */
+  *err = file_error(wth->fh, err_info);
   return -1;
 }
 
diff --git a/wiretap/vms.c b/wiretap/vms.c
index a6f4899cbb..36db1def18 100644
--- a/wiretap/vms.c
+++ b/wiretap/vms.c
@@ -155,43 +155,33 @@ static gboolean parse_vms_rec_hdr(FILE_T fh, struct wtap_pkthdr *phdr,
 
 #ifdef TCPIPTRACE_FRAGMENTS_HAVE_HEADER_LINE
 /* Seeks to the beginning of the next packet, and returns the
-   byte offset.  Returns -1 on failure, and sets "*err" to the error. */
-static long vms_seek_next_packet(wtap *wth, int *err)
+   byte offset.  Returns -1 on failure, and sets "*err" to the error
+   and sets "*err_info" to null or an additional error string. */
+static long vms_seek_next_packet(wtap *wth, int *err, gchar **err_info)
 {
-  long cur_off;
-  char buf[VMS_LINE_LENGTH];
+    long cur_off;
+    char buf[VMS_LINE_LENGTH];
   
-  while (1) {
-    cur_off = file_tell(wth->fh);
-    if (cur_off == -1) {
-      /* Error */
-      *err = file_error(wth->fh, err_info);
-      hdr = NULL;
-      return -1;
-    }
-    if (file_gets(buf, sizeof(buf), wth->fh) != NULL) {
-      if (strstr(buf, VMS_REC_MAGIC_STR1) ||
-	  strstr(buf, VMS_REC_MAGIC_STR2) ||
-	  strstr(buf, VMS_REC_MAGIC_STR2)) {
-		  g_strlcpy(hdr, buf,VMS_LINE_LENGTH);
-		  return cur_off;
-      }
-    } else {
-      if (file_eof(wth->fh)) {
-	/* We got an EOF. */
-	*err = 0;
-      } else {
-	/* We (presumably) got an error (there's no
-	   equivalent to "ferror()" in zlib, alas,
-	   so we don't have a wrapper to check for
-	   an error). */
-	*err = file_error(wth->fh, err_info);
-      }
-      break;
+    while (1) {
+        cur_off = file_tell(wth->fh);
+        if (cur_off == -1) {
+            /* Error */
+            *err = file_error(wth->fh, err_info);
+            return -1;
+        }
+        if (file_gets(buf, sizeof(buf), wth->fh) == NULL) {
+            /* EOF or error. */
+            *err = file_error(wth->fh, err_info);
+            break;
+        }
+        if (strstr(buf, VMS_REC_MAGIC_STR1) ||
+            strstr(buf, VMS_REC_MAGIC_STR2) ||
+            strstr(buf, VMS_REC_MAGIC_STR2)) {
+            g_strlcpy(hdr, buf,VMS_LINE_LENGTH);
+            return cur_off;
+        }
     }
-  }
-  hdr = NULL;
-  return -1;
+    return -1;
 }
 #endif /* TCPIPTRACE_FRAGMENTS_HAVE_HEADER_LINE */
 
@@ -207,50 +197,46 @@ static long vms_seek_next_packet(wtap *wth, int *err)
  */
 static gboolean vms_check_file_type(wtap *wth, int *err, gchar **err_info)
 {
-  char	buf[VMS_LINE_LENGTH];
-  guint	reclen, line;
-  gint64 mpos;
+    char buf[VMS_LINE_LENGTH];
+    guint reclen, line;
+    gint64 mpos;
   
-  buf[VMS_LINE_LENGTH-1] = '\0';
+    buf[VMS_LINE_LENGTH-1] = '\0';
   
-  for (line = 0; line < VMS_HEADER_LINES_TO_CHECK; line++) {
-    mpos = file_tell(wth->fh);
-    if (mpos == -1) {
-      /* Error. */
-      *err = file_error(wth->fh, err_info);
-      return FALSE;
-    }
-    if (file_gets(buf, VMS_LINE_LENGTH, wth->fh) != NULL) {
+    for (line = 0; line < VMS_HEADER_LINES_TO_CHECK; line++) {
+        mpos = file_tell(wth->fh);
+        if (mpos == -1) {
+            /* Error. */
+            *err = file_error(wth->fh, err_info);
+            return FALSE;
+        }
+        if (file_gets(buf, VMS_LINE_LENGTH, wth->fh) == NULL) {
+            /* EOF or error. */
+            *err = file_error(wth->fh, err_info);
+            return FALSE;
+        }
       
-      reclen = (guint) strlen(buf);
-      if (reclen < strlen(VMS_HDR_MAGIC_STR1) ||
-	  reclen < strlen(VMS_HDR_MAGIC_STR2) || 
-	  reclen < strlen(VMS_HDR_MAGIC_STR3)) {
-	continue;
-      }
+        reclen = (guint) strlen(buf);
+        if (reclen < strlen(VMS_HDR_MAGIC_STR1) ||
+            reclen < strlen(VMS_HDR_MAGIC_STR2) || 
+            reclen < strlen(VMS_HDR_MAGIC_STR3)) {
+            continue;
+        }
       
-      if (strstr(buf, VMS_HDR_MAGIC_STR1) ||
-	  strstr(buf, VMS_HDR_MAGIC_STR2) ||
-	  strstr(buf, VMS_HDR_MAGIC_STR3)) {
-	/* Go back to the beginning of this line, so we will
-	 * re-read it. */
-	if (file_seek(wth->fh, mpos, SEEK_SET, err) == -1) {
-	  /* Error. */
-	  return FALSE;
-	}
-	return TRUE;
-      }
-    } else {
-      /* EOF or error. */
-      if (file_eof(wth->fh))
-	*err = 0;
-      else
-	*err = file_error(wth->fh, err_info);
-      return FALSE;
+        if (strstr(buf, VMS_HDR_MAGIC_STR1) ||
+            strstr(buf, VMS_HDR_MAGIC_STR2) ||
+            strstr(buf, VMS_HDR_MAGIC_STR3)) {
+            /* Go back to the beginning of this line, so we will
+             * re-read it. */
+            if (file_seek(wth->fh, mpos, SEEK_SET, err) == -1) {
+                /* Error. */
+                return FALSE;
+            }
+            return TRUE;
+        }
     }
-  }
-  *err = 0;
-  return FALSE;
+    *err = 0;
+    return FALSE;
 }
 
 
@@ -268,7 +254,7 @@ int vms_open(wtap *wth, int *err, gchar **err_info)
     wth->snapshot_length = 0; /* not known */
     wth->subtype_read = vms_read;
     wth->subtype_seek_read = vms_seek_read;
-	wth->tsprecision = WTAP_FILE_TSPREC_CSEC;
+    wth->tsprecision = WTAP_FILE_TSPREC_CSEC;
 
     return 1;
 }
@@ -282,7 +268,7 @@ static gboolean vms_read(wtap *wth, int *err, gchar **err_info,
 
     /* Find the next packet */
 #ifdef TCPIPTRACE_FRAGMENTS_HAVE_HEADER_LINE
-    offset = vms_seek_next_packet(wth, err);
+    offset = vms_seek_next_packet(wth, err, err_info);
 #else
     offset = file_tell(wth->fh);
 #endif