Greetings Martin, all.

Attached is a patch that partly rewrites the check_compat_test method, taking the points raised below into consideration.

Changelog:
        * util/output.cpp (check_compat_test): Rewrite FSM to eliminate seek
        to near end (was causing parsing issues on tests with output
        following result block).

Martin Sebor wrote:
Andrew Black wrote:
One probable reason is that some legacy tests (like 27_iosfile) produce multiple assertion totals (or are supposed to), and we want the last of these, rather than the first.

To handle this case we should process the file from the beginning
and take the last total as the final one.

A second possible reason was to make the parsing FSM slightly simpler (though adding an additional character or two to the pattern shouldn't make it much more complex).

I don't think avoiding the seek would add too much complexity
(the default non-compat case doesn't seek and it's no more
complex than the compat mode).

A final possible reason was to make the parsing faster for long output files.

If the common case is the default mode (i.e., not --compat)
we optimized the wrong branch :) In any case, getting correct
results slowly is better than getting the wrong results fast.

I suggest you rewrite check_compat_test() to avoid seeking
and scan the whole file, top to bottom, and take the last
total.

Martin

Index: util/output.cpp
===================================================================
--- util/output.cpp	(revision 515677)
+++ util/output.cpp	(working copy)
@@ -138,16 +138,14 @@
 static void
 check_compat_test (FILE* data, struct target_status* status)
 {
-    int read = 0;
+    bool read = 0;
     unsigned fsm = 0;
     char tok;
 
     assert (0 != data);
     assert (0 != status);
 
-    fseek (data, -70, SEEK_END); /* Seek near the end of the file */
-
-    for (tok = fgetc (data); fsm < 4 && !feof (data); tok = fgetc (data)) {
+    for (tok = fgetc (data); !feof (data); tok = fgetc (data)) {
         switch (tok) {
         case '\n':
             fsm = 1;
@@ -159,23 +157,22 @@
                 fsm = 0;
             break;
         case ' ':
-            if (3 == fsm) {
+            if (3 == fsm) 
                 ++fsm;
-                break;
-            }
+            else
+                fsm = 0;
+            break;
+        case 'W':
+            if (4 == fsm && !feof (data)) /* leading "## W" eaten */
+                read = 3 == fscanf (data, "arnings = %u\n## Assertions = %u\n"
+                       "## FailedAssertions = %u",
+                       &status->t_warn, &status->assert, &status->failed);
         default:
             fsm = 0;
         }
     }
-    if (!feof (data)) { /* leading "## W" eaten above */
-        read = fscanf (data, "arnings = %u\n## Assertions = %u\n"
-                       "## FailedAssertions = %u",
-                       &status->t_warn, &status->assert, &status->failed);
-    }
-
-    if (3 != read) {
+    if (!read)
         status->status = ST_FORMAT;
-    }
 }
 
 /**

Reply via email to