You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@stdcxx.apache.org by ab...@apache.org on 2008/04/08 19:12:36 UTC

svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Author: ablack
Date: Tue Apr  8 10:12:34 2008
New Revision: 646002

URL: http://svn.apache.org/viewvc?rev=646002&view=rev
Log:
2008-04-08  Andrew Black  <ab...@roguewave.com>

	STDCXX-426
	* util/output.cpp: Revert r643120, r644314, r645002, and
	r645445. The initial version of the patch for STDCXX-426 (r643120)
	resulted in failed processing of test output on windows. The
	subsequent changes were efforts to resolve this failure, but
	resulted either in failed example diffing on windows, or failed to
	resolve the observed misbehavior.

Modified:
    stdcxx/trunk/util/output.cpp

Modified: stdcxx/trunk/util/output.cpp
URL: http://svn.apache.org/viewvc/stdcxx/trunk/util/output.cpp?rev=646002&r1=646001&r2=646002&view=diff
==============================================================================
--- stdcxx/trunk/util/output.cpp (original)
+++ stdcxx/trunk/util/output.cpp Tue Apr  8 10:12:34 2008
@@ -44,239 +44,6 @@
 #  define ENOENT 2
 #endif   /* ENOENT */
 
-/**
-   Arbitrary constant controling static read buffer size.
-
-   @see check_example ()
-   @see rbread ()
-*/
-#define DELTA_BUF_LEN 64
-
-/** 
-    This structure is used to encapsulate the data involved in a backwards
-    file read.
-*/
-struct readback {
-    FILE* src;
-    char buf[DELTA_BUF_LEN];
-    long bpos;
-};
-
-/**
-   Initializes the provided readback structure, with the provided file
-   handle.
-
-   @param rb pointer to readback structure to initialize
-   @param src file handle to initialize rb with.
-   @returns true if successfully initialized, false otherwise.
-*/
-static bool
-rbinit (struct readback* rb, FILE* const src)
-{
-    const size_t buflen = sizeof rb->buf;
-    long fpos;
-    assert (0 != rb);
-    assert (0 != src);
-
-    rb->src = src;
-    if (-1 == fseek (rb->src, 0, SEEK_END))
-        return false;
-    fpos = ftell (rb->src);
-    if (-1 == fpos)
-        return false;
-
-    if (fpos <= buflen)
-        rb->bpos = fpos;
-    else
-        rb->bpos = buflen;
-
-    if (-1 == fseek (rb->src, fpos - rb->bpos, SEEK_SET))
-        return false;
-
-    return rb->bpos == fread (rb->buf, 1, rb->bpos, rb->src);
-}
-
-/** 
-   This method is semi-analagous to feof.
-   
-   @param rb pointer to readback structure to check begin-of-file state for
-   @returns true if structure points to the begining of the file, false 
-   otherwise
-*/
-static bool
-rbbof (const struct readback* const rb)
-{
-    assert (0 != rb);
-    assert (0 != rb->src);
-    return (0 == rb->bpos) && (0 == ftell (rb->src));
-}
-
-/**
-   Syncronizes the file handle underlying a readback structure to the
-   pointer in the structure.  This method is called if you wish to start
-   reading forward from the current point in the readback structure.
-
-   @param rb pointer to the readback structure to syncronize.
-   @return true if the structure was successfully syncronized, false 
-   otherwise.
-*/
-static bool
-rbsync (struct readback* const rb)
-{
-    assert (0 != rb);
-    assert (0 != rb->src);
-    const size_t buflen = sizeof rb->buf;
-    long fpos = ftell (rb->src) - buflen;
-    if (0 > fpos)
-        fpos=0;
-    fpos += rb->bpos;
-    rb->bpos=0;
-
-    return -1 != fseek (rb->src, fpos, SEEK_SET);
-}
-
-/**
-   Updates the provided readback structure, returning the last unread 
-   character in the file.  This method is semi-analagous to fgetc.
-   
-   @param rb pointer to readback structure to read from.
-   @return EOF if beginning of file or I/O error, read character
-   otherwise.
-*/
-static char
-rbgetc (struct readback* rb)
-{
-    const size_t buflen = sizeof rb->buf;
-    assert (0 != rb);
-    assert (0 != rb->src);
-    if (!rb->bpos) {
-        const size_t bufdelta = buflen << 1;
-        long fpos = ftell (rb->src);
-        long seek;
-
-        if (-1 == fpos)
-            return EOF;
-
-        if (bufdelta <= fpos) {
-            seek = fpos - bufdelta;
-            rb->bpos = buflen;
-        }
-        else if (buflen < fpos) {
-            seek = 0;
-            rb->bpos = fpos - buflen;
-        }
-        else {
-            fseek (rb->src, 0, SEEK_SET);
-            return EOF;
-        }
-
-        if (-1 == fseek (rb->src, seek, SEEK_SET))
-            return EOF;
-
-        if (rb->bpos != fread (rb->buf, 1, rb->bpos, rb->src))
-            return EOF;
-    }
-    assert (0 < rb->bpos && rb->bpos <= buflen);
-
-    return rb->buf [--rb->bpos];
-}
-
-/**
-   This method is semi-analagous to fscanf, with some key differences.
-   First, it opperates on a readback structure.  Second, it scans until
-   either the search pattern is matched, or until the begining of the file
-   is reached.  Third, it reads backwards from the end of the file.
-
-   Limitations: string to match can't end in a repeated set of characters
-   (ie: 'singing' has the repeated characters 'ing')
-
-   @param rb pointer to readback structure to operate on
-   @param match string to search for
-   @return true if match is found, false if begining of file is reached
-*/
-static bool
-rbscanf (struct readback* rb, const char* const match)
-{
-    char tok;
-    size_t matched;
-    size_t count;
-    assert (0 != rb);
-    assert (0 != match);
-
-    matched = count = strlen (match) - 1;
-    
-    for(tok = rbgetc(rb); matched && !rbbof (rb); tok = rbgetc (rb)) {
-        if (tok == match [matched])
-            --matched;
-        else
-            matched = count;
-    }
-    return !matched;
-}
-
-/**
-   This method is semi-analagous to fscanf, reading backwards on a readback
-   structure, starting from the end of the file.
-
-   This method searches backwards for the first (last) number and captures
-   it.  It then checks if the match string exists directly after (before)
-   the captured value.  If this is the case, a match is considered to have
-   been found, and the captured value is copied to val.  Otherwise, the 
-   search process is restarted.  If the search hits the beginning of the 
-   file, matching is considered to have failed, and val is unaltered.
-
-   Limitations: string to match can't end in a repeated set of characters
-   (ie: 'singing' has the repeated characters 'ing'.) or a repeated set
-   of characters, bordering on either side of one or more numbers (ie: 
-   'xyzzy9zzy' has the repeated characters 'zzy')
-
-   @param rb pointer to readback structure to operate on.
-   @param match string to search for.
-   @param val reference to variable to store captured value in.
-   @return true if match is found, false if begining of file is reached.
-*/
-static bool
-rbscanf (struct readback* rb, const char* const match, unsigned& val)
-{
-    size_t count;
-    unsigned tval =0;
-    unsigned radix;
-    assert (0 != rb);
-    assert (0 != match);
-
-    count = strlen (match) - 1;
-    
-    while (!rbbof(rb)) {
-        char tok;
-        size_t matched = count;
-
-        tval=0;
-        radix=1;
-
-        /* Search for a numeric digit */
-        for (tok = rbgetc (rb); 
-            (tok < '0' || tok > '9') && !rbbof (rb); 
-             tok = rbgetc (rb)) /* Do nothing */;
-
-        /* Read in the number. */
-        for ( ; tok >= '0' && tok <= '9' && !rbbof (rb); 
-            tok = rbgetc (rb)) {
-            tval += (tok-'0') * radix;
-            radix *= 10;
-        }
-
-        /* Make certain the content prior to the number in the file 
-           matches the search pattern. */
-        for( ; matched && tok == match [matched] && !rbbof (rb); 
-            tok = rbgetc (rb), --matched) /* Do nothing */;
-
-        if (tok == match [matched]) {
-            val = tval;
-            return true;
-        }
-    }
-    return false;
-}
 
 /**
    Parses contents of the open file handle data for test target_name.
@@ -291,7 +58,6 @@
 static void
 check_test (FILE* data, struct target_status* status)
 {
-    struct readback buf;
     unsigned r_lvl    = 0;   /* diagnostic severity level */
     unsigned r_active = 0;   /* number of active diagnostics */
     unsigned r_total  = 0;   /* total number of diagnostics */
@@ -307,28 +73,15 @@
     assert (0 != data);
     assert (0 != status);
 
-    if (!rbinit (&buf, data)) {
-        status->status = ST_SYSTEM_ERROR;
-        return;
-    }
+    tok = fgetc (data);
 
-    if (rbbof (&buf)) {
+    if (feof (data)) {
         /* target produced no output (regression test?) */
         status->status = ST_NO_OUTPUT;
         return;
     }
 
-    if (!rbscanf (&buf, "| INACTIVE |")) {
-        status->status = ST_FORMAT;
-        return;
-    }
-
-    rbsync(&buf);
-
-    /* While it'd probably be (slightly) faster to seek to a fixed position 
-       after the file pointer has been synced, this should be more reliable 
-    */
-    for (tok = fgetc (data); fsm < 6 && !feof (data); tok = fgetc (data)) {
+    for ( ; fsm < 6 && !feof (data); tok = fgetc (data)) {
         switch (tok) {
         case '\n':
             fsm = 1;
@@ -397,25 +150,58 @@
 static void
 check_compat_test (FILE* data, struct target_status* status)
 {
-    struct readback buf;
+    int read = 0;
+    unsigned fsm = 0;
+    char tok;
 
     assert (0 != data);
     assert (0 != status);
 
-    if (!rbinit (&buf, data)) {
-        status->status = ST_SYSTEM_ERROR;
-    } 
-    else if (rbbof (&buf)) {
+    tok = fgetc (data);
+
+    if (feof (data)) {
         /* target produced no output (regression test?) */
         status->status = ST_NO_OUTPUT;
+        return;
     }
-    else if (   !rbscanf (&buf, "## FailedAssertions = ", status->failed)
-             || !rbscanf (&buf, "## Assertions = ", status->assert)
-	     || !rbscanf (&buf, "## Warnings = ", status->t_warn)) {
+
+    for ( ; !feof (data); tok = fgetc (data)) {
+        switch (tok) {
+        case '\n':
+            fsm = 1;
+            break;
+        case '#':
+            if (1 == fsm || 2 == fsm)
+                ++fsm;
+            else
+                fsm = 0;
+            break;
+        case ' ':
+            if (3 == fsm) 
+                ++fsm;
+            else
+                fsm = 0;
+            break;
+        case 'W':
+            if (4 == fsm && !feof (data)) /* leading "## W" eaten */
+                read = fscanf (data, "arnings = %u\n## Assertions = %u\n"
+                       "## FailedAssertions = %u",
+                       &status->t_warn, &status->assert, &status->failed);
+        default:
+            fsm = 0;
+        }
+    }
+    if (3 != read) {
         status->status = ST_FORMAT;
     }
 }
 
+/**
+   Arbitrary constant controling static read buffer size.
+
+   @see check_example ()
+*/
+#define DELTA_BUF_LEN 64
 
 /**
    Parses output file out_name for the example target_name.
@@ -483,7 +269,7 @@
         }
     }
 
-    fref = fopen (ref_name, "rb");
+    fref = fopen (ref_name, "r");
 
     if (0 == fref) {
         int cache = errno; /* caching errno, as puts could alter it */
@@ -548,7 +334,7 @@
     assert (0 != options->argv [0]);
     out_name = output_name (options->argv [0]);
 
-    data = fopen (out_name, "rb");
+    data = fopen (out_name, "r");
 
     if (0 == data) {
         if (ENOENT != errno)



RE: svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Eric Lemings wrote:
> 
>Does this mean we have no solution for resolving this issue (i.e.
>back to Square One as it were)?  It's currently classified as one
>of two Critical issues for 4.2.1 and Andrew's last comment is
>"Further work is necessary".  Question is, how much work?
>
>Thanks,
>Brad.
>

I don't know. I'm having a hard time seeing why we were getting the DIFF
errors with the examples in the nightly builds. I know for a fact that I
tested the test, examples and locales multiple times before I committed
my last 'fix', and I've just rebuilt using the code from yesterday and I
have no problems today either...

C:\Build\stdcxx\trunk\msvc-8.0\11s\examples>..\bin\exec.exe -d
c:\build\stdcxx\trunk\examples @aa
NAME                                     STATUS WARN ASSERTS FAILED
PERCNT    USER     SYS    REAL
accumulate.exe                                0    0
0.015   0.000   0.203
adj_diff.exe                                  0    0
0.015   0.000   0.187
advance.exe                                   0    0
0.015   0.000   0.281
alg1.exe                                      0    0
0.015   0.000   0.124
alg2.exe                                      0    0
0.015   0.000   0.109
alg3.exe                                      0    0
0.015   0.015   0.249
alg4.exe                                      0    0
0.015   0.000   0.234
alg5.exe                                      0    0
0.015   0.000   0.234
alg6.exe                                      0    0
0.015   0.000   0.140
alg7.exe                                      0    0
0.015   0.000   0.124
<snip>


I'm certain that this boils down to check_test() and check_compat_test()
functions needing the provided file to be opened in binary mode and the
check_example() function needing it to be text mode. I'm uncertain why
binary mode doesn't work in the nightly builds, but works fine locally,
but it appears that we previously used text mode for comparing the
example output. If the problem is that simple, then I the parse_output()
function could have been augmented to workaround the problem.

+    const bool is_not_an_example =
+        0 == options->data_dir || '\0' == *options->data_dir;
+
+    const char* file_mode = "r";
+    if (iis_not_an_example)
+        file_mode = "rb";
+
+    data = fopen (out_name, file_mode);
-    data = fopen (out_name, "rb");

     if (0 == data) {
         if (ENOENT != errno)
             warn ("Error opening %s: %s\n", out_name, strerror
(errno));
         status->status = ST_NO_OUTPUT;
     }
     else {
+        if (is_not_an_example) {
-        if (0 == options->data_dir || '\0' == *options->data_dir) {

This opens the file in binary mode for the tests and locales, and in
text mode for the examples.

Travis

RE: svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Martin Sebor wrote:
>
>Has anyone tested or looked at the new patch? Travis?
>

I have not tested it, but I pointed out that it still has problems in my
previous post in this thread... http://tinyurl.com/4pyr4j. I mention
that the msdn docs say you can only reliably use fseek() to move to the
origins [begin and end of file] or to a value previously attained from a
call to ftell().

Andrew's patch uses fseek() in a manner that is inconsistent with both
these requirements in several places [rbinit(), rbsync(), and rbgetc()].
IMO, this makes the patch no good.

That said, I'm running a test on with a regular windows build just to be
sure. Even if that works, to be absolutely certain that this fix works
I'll have to emulate the nightly build system get the entire source
heirarchy on windows system with unix line ends.

Travis

>Martin
>
>Andrew Black wrote:
>> Not a whole lot of work it seems.
>> 
>> I've been looking into this problem this afternoon, and I 
>> believe I've determined a solution.
>> 

Re: svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Posted by Martin Sebor <se...@roguewave.com>.
Has anyone tested or looked at the new patch? Travis?

Martin

Andrew Black wrote:
> Not a whole lot of work it seems.
> 
> I've been looking into this problem this afternoon, and I believe I've
> determined a solution.
> 
> The initial issue (which Farid spotted) was that windows has an fread
> behavior which differers in subtle ways from that on unix systems.  In
> particular, it performs CRLF to CR translation when it reads a file in
> text mode, and as a result the number of characters read into the buffer
> is less than the number of bytes which the operating system was asked to
> read. This differs from the POSIX spec which reads in part 'Upon
> successful completion, fread() shall return the number of elements
> successfully read /which is less than nitems only if a read error or
> end-of-file is encountered/.' (My emphasis on the second half of the quote).
> 
> The solution (as implemented by Farid and Travis) was to read the input
> files in binary mode, rather than text mode. The problem with this
> solution is that it causes differences in line endings to show up as
> differences where this is none.  These differences in line endings are
> introduced by the creation of the nightly testing archive (on a unix
> system with unix line endings), but the run being performed on a windows
> system (with the output using windows line endings).
> 
> My solution (attached) removes the checks on the return from fread, and
> saves that value in the readback structure as the pointer into the read
> buffer.
> 
> --Andrew Black
> 
> Eric Lemings wrote:
>>  
>> Does this mean we have no solution for resolving this issue (i.e.
>> back to Square One as it were)?  It's currently classified as one
>> of two Critical issues for 4.2.1 and Andrew's last comment is
>> "Further work is necessary".  Question is, how much work?
>>
>> Thanks,
>> Brad.
>>
>>> -----Original Message-----
>>> From: ablack@apache.org [mailto:ablack@apache.org] 
>>> Sent: Tuesday, April 08, 2008 11:13 AM
>>> To: commits@stdcxx.apache.org
>>> Subject: svn commit: r646002 - /stdcxx/trunk/util/output.cpp
>>>
>>> Author: ablack
>>> Date: Tue Apr  8 10:12:34 2008
>>> New Revision: 646002
>>>
>>> URL: http://svn.apache.org/viewvc?rev=646002&view=rev
>>> Log:
>>> 2008-04-08  Andrew Black  <ab...@roguewave.com>
>>>
>>> 	STDCXX-426
>>> 	* util/output.cpp: Revert r643120, r644314, r645002, and
>>> 	r645445. The initial version of the patch for 
>>> STDCXX-426 (r643120)
>>> 	resulted in failed processing of test output on windows. The
>>> 	subsequent changes were efforts to resolve this failure, but
>>> 	resulted either in failed example diffing on windows, 
>>> or failed to
>>> 	resolve the observed misbehavior.
>>>
>>> Modified:
>>>     stdcxx/trunk/util/output.cpp
>>>
>>> Modified: stdcxx/trunk/util/output.cpp
>>> URL: 
>>> http://svn.apache.org/viewvc/stdcxx/trunk/util/output.cpp?rev=
>>> 646002&r1=646001&r2=646002&view=diff
>>> ==============================================================
>>> ================
>>> --- stdcxx/trunk/util/output.cpp (original)
>>> +++ stdcxx/trunk/util/output.cpp Tue Apr  8 10:12:34 2008
>>> @@ -44,239 +44,6 @@
>>>  #  define ENOENT 2
>>>  #endif   /* ENOENT */
>>>  
>>> -/**
>>> -   Arbitrary constant controling static read buffer size.
>>> -
>>> -   @see check_example ()
>>> -   @see rbread ()
>>> -*/
>>> -#define DELTA_BUF_LEN 64
>>> -
>>> -/** 
>>> -    This structure is used to encapsulate the data involved 
>>> in a backwards
>>> -    file read.
>>> -*/
>>> -struct readback {
>>> -    FILE* src;
>>> -    char buf[DELTA_BUF_LEN];
>>> -    long bpos;
>>> -};
>>> -
>>> -/**
>>> -   Initializes the provided readback structure, with the 
>>> provided file
>>> -   handle.
>>> -
>>> -   @param rb pointer to readback structure to initialize
>>> -   @param src file handle to initialize rb with.
>>> -   @returns true if successfully initialized, false otherwise.
>>> -*/
>>> -static bool
>>> -rbinit (struct readback* rb, FILE* const src)
>>> -{
>>> -    const size_t buflen = sizeof rb->buf;
>>> -    long fpos;
>>> -    assert (0 != rb);
>>> -    assert (0 != src);
>>> -
>>> -    rb->src = src;
>>> -    if (-1 == fseek (rb->src, 0, SEEK_END))
>>> -        return false;
>>> -    fpos = ftell (rb->src);
>>> -    if (-1 == fpos)
>>> -        return false;
>>> -
>>> -    if (fpos <= buflen)
>>> -        rb->bpos = fpos;
>>> -    else
>>> -        rb->bpos = buflen;
>>> -
>>> -    if (-1 == fseek (rb->src, fpos - rb->bpos, SEEK_SET))
>>> -        return false;
>>> -
>>> -    return rb->bpos == fread (rb->buf, 1, rb->bpos, rb->src);
>>> -}
>>> -
>>> -/** 
>>> -   This method is semi-analagous to feof.
>>> -   
>>> -   @param rb pointer to readback structure to check 
>>> begin-of-file state for
>>> -   @returns true if structure points to the begining of the 
>>> file, false 
>>> -   otherwise
>>> -*/
>>> -static bool
>>> -rbbof (const struct readback* const rb)
>>> -{
>>> -    assert (0 != rb);
>>> -    assert (0 != rb->src);
>>> -    return (0 == rb->bpos) && (0 == ftell (rb->src));
>>> -}
>>> -
>>> -/**
>>> -   Syncronizes the file handle underlying a readback structure to the
>>> -   pointer in the structure.  This method is called if you 
>>> wish to start
>>> -   reading forward from the current point in the readback structure.
>>> -
>>> -   @param rb pointer to the readback structure to syncronize.
>>> -   @return true if the structure was successfully syncronized, false 
>>> -   otherwise.
>>> -*/
>>> -static bool
>>> -rbsync (struct readback* const rb)
>>> -{
>>> -    assert (0 != rb);
>>> -    assert (0 != rb->src);
>>> -    const size_t buflen = sizeof rb->buf;
>>> -    long fpos = ftell (rb->src) - buflen;
>>> -    if (0 > fpos)
>>> -        fpos=0;
>>> -    fpos += rb->bpos;
>>> -    rb->bpos=0;
>>> -
>>> -    return -1 != fseek (rb->src, fpos, SEEK_SET);
>>> -}
>>> -
>>> -/**
>>> -   Updates the provided readback structure, returning the 
>>> last unread 
>>> -   character in the file.  This method is semi-analagous to fgetc.
>>> -   
>>> -   @param rb pointer to readback structure to read from.
>>> -   @return EOF if beginning of file or I/O error, read character
>>> -   otherwise.
>>> -*/
>>> -static char
>>> -rbgetc (struct readback* rb)
>>> -{
>>> -    const size_t buflen = sizeof rb->buf;
>>> -    assert (0 != rb);
>>> -    assert (0 != rb->src);
>>> -    if (!rb->bpos) {
>>> -        const size_t bufdelta = buflen << 1;
>>> -        long fpos = ftell (rb->src);
>>> -        long seek;
>>> -
>>> -        if (-1 == fpos)
>>> -            return EOF;
>>> -
>>> -        if (bufdelta <= fpos) {
>>> -            seek = fpos - bufdelta;
>>> -            rb->bpos = buflen;
>>> -        }
>>> -        else if (buflen < fpos) {
>>> -            seek = 0;
>>> -            rb->bpos = fpos - buflen;
>>> -        }
>>> -        else {
>>> -            fseek (rb->src, 0, SEEK_SET);
>>> -            return EOF;
>>> -        }
>>> -
>>> -        if (-1 == fseek (rb->src, seek, SEEK_SET))
>>> -            return EOF;
>>> -
>>> -        if (rb->bpos != fread (rb->buf, 1, rb->bpos, rb->src))
>>> -            return EOF;
>>> -    }
>>> -    assert (0 < rb->bpos && rb->bpos <= buflen);
>>> -
>>> -    return rb->buf [--rb->bpos];
>>> -}
>>> -
>>> -/**
>>> -   This method is semi-analagous to fscanf, with some key 
>>> differences.
>>> -   First, it opperates on a readback structure.  Second, it 
>>> scans until
>>> -   either the search pattern is matched, or until the 
>>> begining of the file
>>> -   is reached.  Third, it reads backwards from the end of the file.
>>> -
>>> -   Limitations: string to match can't end in a repeated set 
>>> of characters
>>> -   (ie: 'singing' has the repeated characters 'ing')
>>> -
>>> -   @param rb pointer to readback structure to operate on
>>> -   @param match string to search for
>>> -   @return true if match is found, false if begining of file 
>>> is reached
>>> -*/
>>> -static bool
>>> -rbscanf (struct readback* rb, const char* const match)
>>> -{
>>> -    char tok;
>>> -    size_t matched;
>>> -    size_t count;
>>> -    assert (0 != rb);
>>> -    assert (0 != match);
>>> -
>>> -    matched = count = strlen (match) - 1;
>>> -    
>>> -    for(tok = rbgetc(rb); matched && !rbbof (rb); tok = 
>>> rbgetc (rb)) {
>>> -        if (tok == match [matched])
>>> -            --matched;
>>> -        else
>>> -            matched = count;
>>> -    }
>>> -    return !matched;
>>> -}
>>> -
>>> -/**
>>> -   This method is semi-analagous to fscanf, reading 
>>> backwards on a readback
>>> -   structure, starting from the end of the file.
>>> -
>>> -   This method searches backwards for the first (last) 
>>> number and captures
>>> -   it.  It then checks if the match string exists directly 
>>> after (before)
>>> -   the captured value.  If this is the case, a match is 
>>> considered to have
>>> -   been found, and the captured value is copied to val.  
>>> Otherwise, the 
>>> -   search process is restarted.  If the search hits the 
>>> beginning of the 
>>> -   file, matching is considered to have failed, and val is unaltered.
>>> -
>>> -   Limitations: string to match can't end in a repeated set 
>>> of characters
>>> -   (ie: 'singing' has the repeated characters 'ing'.) or a 
>>> repeated set
>>> -   of characters, bordering on either side of one or more 
>>> numbers (ie: 
>>> -   'xyzzy9zzy' has the repeated characters 'zzy')
>>> -
>>> -   @param rb pointer to readback structure to operate on.
>>> -   @param match string to search for.
>>> -   @param val reference to variable to store captured value in.
>>> -   @return true if match is found, false if begining of file 
>>> is reached.
>>> -*/
>>> -static bool
>>> -rbscanf (struct readback* rb, const char* const match, unsigned& val)
>>> -{
>>> -    size_t count;
>>> -    unsigned tval =0;
>>> -    unsigned radix;
>>> -    assert (0 != rb);
>>> -    assert (0 != match);
>>> -
>>> -    count = strlen (match) - 1;
>>> -    
>>> -    while (!rbbof(rb)) {
>>> -        char tok;
>>> -        size_t matched = count;
>>> -
>>> -        tval=0;
>>> -        radix=1;
>>> -
>>> -        /* Search for a numeric digit */
>>> -        for (tok = rbgetc (rb); 
>>> -            (tok < '0' || tok > '9') && !rbbof (rb); 
>>> -             tok = rbgetc (rb)) /* Do nothing */;
>>> -
>>> -        /* Read in the number. */
>>> -        for ( ; tok >= '0' && tok <= '9' && !rbbof (rb); 
>>> -            tok = rbgetc (rb)) {
>>> -            tval += (tok-'0') * radix;
>>> -            radix *= 10;
>>> -        }
>>> -
>>> -        /* Make certain the content prior to the number in the file 
>>> -           matches the search pattern. */
>>> -        for( ; matched && tok == match [matched] && !rbbof (rb); 
>>> -            tok = rbgetc (rb), --matched) /* Do nothing */;
>>> -
>>> -        if (tok == match [matched]) {
>>> -            val = tval;
>>> -            return true;
>>> -        }
>>> -    }
>>> -    return false;
>>> -}
>>>  
>>>  /**
>>>     Parses contents of the open file handle data for test target_name.
>>> @@ -291,7 +58,6 @@
>>>  static void
>>>  check_test (FILE* data, struct target_status* status)
>>>  {
>>> -    struct readback buf;
>>>      unsigned r_lvl    = 0;   /* diagnostic severity level */
>>>      unsigned r_active = 0;   /* number of active diagnostics */
>>>      unsigned r_total  = 0;   /* total number of diagnostics */
>>> @@ -307,28 +73,15 @@
>>>      assert (0 != data);
>>>      assert (0 != status);
>>>  
>>> -    if (!rbinit (&buf, data)) {
>>> -        status->status = ST_SYSTEM_ERROR;
>>> -        return;
>>> -    }
>>> +    tok = fgetc (data);
>>>  
>>> -    if (rbbof (&buf)) {
>>> +    if (feof (data)) {
>>>          /* target produced no output (regression test?) */
>>>          status->status = ST_NO_OUTPUT;
>>>          return;
>>>      }
>>>  
>>> -    if (!rbscanf (&buf, "| INACTIVE |")) {
>>> -        status->status = ST_FORMAT;
>>> -        return;
>>> -    }
>>> -
>>> -    rbsync(&buf);
>>> -
>>> -    /* While it'd probably be (slightly) faster to seek to a 
>>> fixed position 
>>> -       after the file pointer has been synced, this should 
>>> be more reliable 
>>> -    */
>>> -    for (tok = fgetc (data); fsm < 6 && !feof (data); tok = 
>>> fgetc (data)) {
>>> +    for ( ; fsm < 6 && !feof (data); tok = fgetc (data)) {
>>>          switch (tok) {
>>>          case '\n':
>>>              fsm = 1;
>>> @@ -397,25 +150,58 @@
>>>  static void
>>>  check_compat_test (FILE* data, struct target_status* status)
>>>  {
>>> -    struct readback buf;
>>> +    int read = 0;
>>> +    unsigned fsm = 0;
>>> +    char tok;
>>>  
>>>      assert (0 != data);
>>>      assert (0 != status);
>>>  
>>> -    if (!rbinit (&buf, data)) {
>>> -        status->status = ST_SYSTEM_ERROR;
>>> -    } 
>>> -    else if (rbbof (&buf)) {
>>> +    tok = fgetc (data);
>>> +
>>> +    if (feof (data)) {
>>>          /* target produced no output (regression test?) */
>>>          status->status = ST_NO_OUTPUT;
>>> +        return;
>>>      }
>>> -    else if (   !rbscanf (&buf, "## FailedAssertions = ", 
>>> status->failed)
>>> -             || !rbscanf (&buf, "## Assertions = ", status->assert)
>>> -	     || !rbscanf (&buf, "## Warnings = ", status->t_warn)) {
>>> +
>>> +    for ( ; !feof (data); tok = fgetc (data)) {
>>> +        switch (tok) {
>>> +        case '\n':
>>> +            fsm = 1;
>>> +            break;
>>> +        case '#':
>>> +            if (1 == fsm || 2 == fsm)
>>> +                ++fsm;
>>> +            else
>>> +                fsm = 0;
>>> +            break;
>>> +        case ' ':
>>> +            if (3 == fsm) 
>>> +                ++fsm;
>>> +            else
>>> +                fsm = 0;
>>> +            break;
>>> +        case 'W':
>>> +            if (4 == fsm && !feof (data)) /* leading "## W" eaten */
>>> +                read = fscanf (data, "arnings = %u\n## 
>>> Assertions = %u\n"
>>> +                       "## FailedAssertions = %u",
>>> +                       &status->t_warn, &status->assert, 
>>> &status->failed);
>>> +        default:
>>> +            fsm = 0;
>>> +        }
>>> +    }
>>> +    if (3 != read) {
>>>          status->status = ST_FORMAT;
>>>      }
>>>  }
>>>  
>>> +/**
>>> +   Arbitrary constant controling static read buffer size.
>>> +
>>> +   @see check_example ()
>>> +*/
>>> +#define DELTA_BUF_LEN 64
>>>  
>>>  /**
>>>     Parses output file out_name for the example target_name.
>>> @@ -483,7 +269,7 @@
>>>          }
>>>      }
>>>  
>>> -    fref = fopen (ref_name, "rb");
>>> +    fref = fopen (ref_name, "r");
>>>  
>>>      if (0 == fref) {
>>>          int cache = errno; /* caching errno, as puts could 
>>> alter it */
>>> @@ -548,7 +334,7 @@
>>>      assert (0 != options->argv [0]);
>>>      out_name = output_name (options->argv [0]);
>>>  
>>> -    data = fopen (out_name, "rb");
>>> +    data = fopen (out_name, "r");
>>>  
>>>      if (0 == data) {
>>>          if (ENOENT != errno)
>>>
>>>
>>>


RE: svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Andrew Black wrote:
>
>Not a whole lot of work it seems.
>
>I've been looking into this problem this afternoon, and I believe I've
>determined a solution.
>
>The initial issue (which Farid spotted) was that windows has an fread
>behavior which differers in subtle ways from that on unix systems.  In
>particular, it performs CRLF to CR translation when it reads a file in
>text mode, and as a result the number of characters read into 
>the buffer
>is less than the number of bytes which the operating system 
>was asked to
>read. This differs from the POSIX spec which reads in part 'Upon
>successful completion, fread() shall return the number of elements
>successfully read /which is less than nitems only if a read error or
>end-of-file is encountered/.' (My emphasis on the second half 
>of the quote).
>
>The solution (as implemented by Farid and Travis) was to read the input
>files in binary mode, rather than text mode. The problem with this
>solution is that it causes differences in line endings to show up as
>differences where this is none.  These differences in line endings are
>introduced by the creation of the nightly testing archive (on a unix
>system with unix line endings), but the run being performed on 
>a windows system (with the output using windows line endings).

Arg! I knew my code worked when I tested it. Now it all makes sense.

>
>My solution (attached) removes the checks on the return from fread, and
>saves that value in the readback structure as the pointer into the read
>buffer.
>

I think the following block [from msdn fseek() documentation] throws
a wrench into the works.

    For streams opened in text mode, fseek and _fseeki64 have limited
    use, because carriage return-linefeed translations can cause fseek
    and _fseeki64to produce unexpected results. The only fseek and
    _fseeki64 operations guaranteed to work on streams opened in text
    mode are: 

      * Seeking with an offset of 0 relative to any of the origin
        values.

      * Seeking from the beginning of the file with an offset value
        returned from a call to ftell when using fseek or _ftelli64
        when using _fseeki64.

The latest patch uses fseek() in a manner that is inconsistent with
these requirements. I'm not totally certain of what the behavior will
be, but I'd guess that it is not good.

If the problem of the example diffs is being caused by opening the diff
file in binary mode, then I suggest we leave it as text for that case.
The pseudo-patch I sent in my previous message should work to address
the problem. It isn't ideal, but it does work and is quite simple.

Travis

>--Andrew Black

Re: svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Posted by Andrew Black <ab...@roguewave.com>.
Not a whole lot of work it seems.

I've been looking into this problem this afternoon, and I believe I've
determined a solution.

The initial issue (which Farid spotted) was that windows has an fread
behavior which differers in subtle ways from that on unix systems.  In
particular, it performs CRLF to CR translation when it reads a file in
text mode, and as a result the number of characters read into the buffer
is less than the number of bytes which the operating system was asked to
read. This differs from the POSIX spec which reads in part 'Upon
successful completion, fread() shall return the number of elements
successfully read /which is less than nitems only if a read error or
end-of-file is encountered/.' (My emphasis on the second half of the quote).

The solution (as implemented by Farid and Travis) was to read the input
files in binary mode, rather than text mode. The problem with this
solution is that it causes differences in line endings to show up as
differences where this is none.  These differences in line endings are
introduced by the creation of the nightly testing archive (on a unix
system with unix line endings), but the run being performed on a windows
system (with the output using windows line endings).

My solution (attached) removes the checks on the return from fread, and
saves that value in the readback structure as the pointer into the read
buffer.

--Andrew Black

Eric Lemings wrote:
>  
> Does this mean we have no solution for resolving this issue (i.e.
> back to Square One as it were)?  It's currently classified as one
> of two Critical issues for 4.2.1 and Andrew's last comment is
> "Further work is necessary".  Question is, how much work?
> 
> Thanks,
> Brad.
> 
>> -----Original Message-----
>> From: ablack@apache.org [mailto:ablack@apache.org] 
>> Sent: Tuesday, April 08, 2008 11:13 AM
>> To: commits@stdcxx.apache.org
>> Subject: svn commit: r646002 - /stdcxx/trunk/util/output.cpp
>>
>> Author: ablack
>> Date: Tue Apr  8 10:12:34 2008
>> New Revision: 646002
>>
>> URL: http://svn.apache.org/viewvc?rev=646002&view=rev
>> Log:
>> 2008-04-08  Andrew Black  <ab...@roguewave.com>
>>
>> 	STDCXX-426
>> 	* util/output.cpp: Revert r643120, r644314, r645002, and
>> 	r645445. The initial version of the patch for 
>> STDCXX-426 (r643120)
>> 	resulted in failed processing of test output on windows. The
>> 	subsequent changes were efforts to resolve this failure, but
>> 	resulted either in failed example diffing on windows, 
>> or failed to
>> 	resolve the observed misbehavior.
>>
>> Modified:
>>     stdcxx/trunk/util/output.cpp
>>
>> Modified: stdcxx/trunk/util/output.cpp
>> URL: 
>> http://svn.apache.org/viewvc/stdcxx/trunk/util/output.cpp?rev=
>> 646002&r1=646001&r2=646002&view=diff
>> ==============================================================
>> ================
>> --- stdcxx/trunk/util/output.cpp (original)
>> +++ stdcxx/trunk/util/output.cpp Tue Apr  8 10:12:34 2008
>> @@ -44,239 +44,6 @@
>>  #  define ENOENT 2
>>  #endif   /* ENOENT */
>>  
>> -/**
>> -   Arbitrary constant controling static read buffer size.
>> -
>> -   @see check_example ()
>> -   @see rbread ()
>> -*/
>> -#define DELTA_BUF_LEN 64
>> -
>> -/** 
>> -    This structure is used to encapsulate the data involved 
>> in a backwards
>> -    file read.
>> -*/
>> -struct readback {
>> -    FILE* src;
>> -    char buf[DELTA_BUF_LEN];
>> -    long bpos;
>> -};
>> -
>> -/**
>> -   Initializes the provided readback structure, with the 
>> provided file
>> -   handle.
>> -
>> -   @param rb pointer to readback structure to initialize
>> -   @param src file handle to initialize rb with.
>> -   @returns true if successfully initialized, false otherwise.
>> -*/
>> -static bool
>> -rbinit (struct readback* rb, FILE* const src)
>> -{
>> -    const size_t buflen = sizeof rb->buf;
>> -    long fpos;
>> -    assert (0 != rb);
>> -    assert (0 != src);
>> -
>> -    rb->src = src;
>> -    if (-1 == fseek (rb->src, 0, SEEK_END))
>> -        return false;
>> -    fpos = ftell (rb->src);
>> -    if (-1 == fpos)
>> -        return false;
>> -
>> -    if (fpos <= buflen)
>> -        rb->bpos = fpos;
>> -    else
>> -        rb->bpos = buflen;
>> -
>> -    if (-1 == fseek (rb->src, fpos - rb->bpos, SEEK_SET))
>> -        return false;
>> -
>> -    return rb->bpos == fread (rb->buf, 1, rb->bpos, rb->src);
>> -}
>> -
>> -/** 
>> -   This method is semi-analagous to feof.
>> -   
>> -   @param rb pointer to readback structure to check 
>> begin-of-file state for
>> -   @returns true if structure points to the begining of the 
>> file, false 
>> -   otherwise
>> -*/
>> -static bool
>> -rbbof (const struct readback* const rb)
>> -{
>> -    assert (0 != rb);
>> -    assert (0 != rb->src);
>> -    return (0 == rb->bpos) && (0 == ftell (rb->src));
>> -}
>> -
>> -/**
>> -   Syncronizes the file handle underlying a readback structure to the
>> -   pointer in the structure.  This method is called if you 
>> wish to start
>> -   reading forward from the current point in the readback structure.
>> -
>> -   @param rb pointer to the readback structure to syncronize.
>> -   @return true if the structure was successfully syncronized, false 
>> -   otherwise.
>> -*/
>> -static bool
>> -rbsync (struct readback* const rb)
>> -{
>> -    assert (0 != rb);
>> -    assert (0 != rb->src);
>> -    const size_t buflen = sizeof rb->buf;
>> -    long fpos = ftell (rb->src) - buflen;
>> -    if (0 > fpos)
>> -        fpos=0;
>> -    fpos += rb->bpos;
>> -    rb->bpos=0;
>> -
>> -    return -1 != fseek (rb->src, fpos, SEEK_SET);
>> -}
>> -
>> -/**
>> -   Updates the provided readback structure, returning the 
>> last unread 
>> -   character in the file.  This method is semi-analagous to fgetc.
>> -   
>> -   @param rb pointer to readback structure to read from.
>> -   @return EOF if beginning of file or I/O error, read character
>> -   otherwise.
>> -*/
>> -static char
>> -rbgetc (struct readback* rb)
>> -{
>> -    const size_t buflen = sizeof rb->buf;
>> -    assert (0 != rb);
>> -    assert (0 != rb->src);
>> -    if (!rb->bpos) {
>> -        const size_t bufdelta = buflen << 1;
>> -        long fpos = ftell (rb->src);
>> -        long seek;
>> -
>> -        if (-1 == fpos)
>> -            return EOF;
>> -
>> -        if (bufdelta <= fpos) {
>> -            seek = fpos - bufdelta;
>> -            rb->bpos = buflen;
>> -        }
>> -        else if (buflen < fpos) {
>> -            seek = 0;
>> -            rb->bpos = fpos - buflen;
>> -        }
>> -        else {
>> -            fseek (rb->src, 0, SEEK_SET);
>> -            return EOF;
>> -        }
>> -
>> -        if (-1 == fseek (rb->src, seek, SEEK_SET))
>> -            return EOF;
>> -
>> -        if (rb->bpos != fread (rb->buf, 1, rb->bpos, rb->src))
>> -            return EOF;
>> -    }
>> -    assert (0 < rb->bpos && rb->bpos <= buflen);
>> -
>> -    return rb->buf [--rb->bpos];
>> -}
>> -
>> -/**
>> -   This method is semi-analagous to fscanf, with some key 
>> differences.
>> -   First, it opperates on a readback structure.  Second, it 
>> scans until
>> -   either the search pattern is matched, or until the 
>> begining of the file
>> -   is reached.  Third, it reads backwards from the end of the file.
>> -
>> -   Limitations: string to match can't end in a repeated set 
>> of characters
>> -   (ie: 'singing' has the repeated characters 'ing')
>> -
>> -   @param rb pointer to readback structure to operate on
>> -   @param match string to search for
>> -   @return true if match is found, false if begining of file 
>> is reached
>> -*/
>> -static bool
>> -rbscanf (struct readback* rb, const char* const match)
>> -{
>> -    char tok;
>> -    size_t matched;
>> -    size_t count;
>> -    assert (0 != rb);
>> -    assert (0 != match);
>> -
>> -    matched = count = strlen (match) - 1;
>> -    
>> -    for(tok = rbgetc(rb); matched && !rbbof (rb); tok = 
>> rbgetc (rb)) {
>> -        if (tok == match [matched])
>> -            --matched;
>> -        else
>> -            matched = count;
>> -    }
>> -    return !matched;
>> -}
>> -
>> -/**
>> -   This method is semi-analagous to fscanf, reading 
>> backwards on a readback
>> -   structure, starting from the end of the file.
>> -
>> -   This method searches backwards for the first (last) 
>> number and captures
>> -   it.  It then checks if the match string exists directly 
>> after (before)
>> -   the captured value.  If this is the case, a match is 
>> considered to have
>> -   been found, and the captured value is copied to val.  
>> Otherwise, the 
>> -   search process is restarted.  If the search hits the 
>> beginning of the 
>> -   file, matching is considered to have failed, and val is unaltered.
>> -
>> -   Limitations: string to match can't end in a repeated set 
>> of characters
>> -   (ie: 'singing' has the repeated characters 'ing'.) or a 
>> repeated set
>> -   of characters, bordering on either side of one or more 
>> numbers (ie: 
>> -   'xyzzy9zzy' has the repeated characters 'zzy')
>> -
>> -   @param rb pointer to readback structure to operate on.
>> -   @param match string to search for.
>> -   @param val reference to variable to store captured value in.
>> -   @return true if match is found, false if begining of file 
>> is reached.
>> -*/
>> -static bool
>> -rbscanf (struct readback* rb, const char* const match, unsigned& val)
>> -{
>> -    size_t count;
>> -    unsigned tval =0;
>> -    unsigned radix;
>> -    assert (0 != rb);
>> -    assert (0 != match);
>> -
>> -    count = strlen (match) - 1;
>> -    
>> -    while (!rbbof(rb)) {
>> -        char tok;
>> -        size_t matched = count;
>> -
>> -        tval=0;
>> -        radix=1;
>> -
>> -        /* Search for a numeric digit */
>> -        for (tok = rbgetc (rb); 
>> -            (tok < '0' || tok > '9') && !rbbof (rb); 
>> -             tok = rbgetc (rb)) /* Do nothing */;
>> -
>> -        /* Read in the number. */
>> -        for ( ; tok >= '0' && tok <= '9' && !rbbof (rb); 
>> -            tok = rbgetc (rb)) {
>> -            tval += (tok-'0') * radix;
>> -            radix *= 10;
>> -        }
>> -
>> -        /* Make certain the content prior to the number in the file 
>> -           matches the search pattern. */
>> -        for( ; matched && tok == match [matched] && !rbbof (rb); 
>> -            tok = rbgetc (rb), --matched) /* Do nothing */;
>> -
>> -        if (tok == match [matched]) {
>> -            val = tval;
>> -            return true;
>> -        }
>> -    }
>> -    return false;
>> -}
>>  
>>  /**
>>     Parses contents of the open file handle data for test target_name.
>> @@ -291,7 +58,6 @@
>>  static void
>>  check_test (FILE* data, struct target_status* status)
>>  {
>> -    struct readback buf;
>>      unsigned r_lvl    = 0;   /* diagnostic severity level */
>>      unsigned r_active = 0;   /* number of active diagnostics */
>>      unsigned r_total  = 0;   /* total number of diagnostics */
>> @@ -307,28 +73,15 @@
>>      assert (0 != data);
>>      assert (0 != status);
>>  
>> -    if (!rbinit (&buf, data)) {
>> -        status->status = ST_SYSTEM_ERROR;
>> -        return;
>> -    }
>> +    tok = fgetc (data);
>>  
>> -    if (rbbof (&buf)) {
>> +    if (feof (data)) {
>>          /* target produced no output (regression test?) */
>>          status->status = ST_NO_OUTPUT;
>>          return;
>>      }
>>  
>> -    if (!rbscanf (&buf, "| INACTIVE |")) {
>> -        status->status = ST_FORMAT;
>> -        return;
>> -    }
>> -
>> -    rbsync(&buf);
>> -
>> -    /* While it'd probably be (slightly) faster to seek to a 
>> fixed position 
>> -       after the file pointer has been synced, this should 
>> be more reliable 
>> -    */
>> -    for (tok = fgetc (data); fsm < 6 && !feof (data); tok = 
>> fgetc (data)) {
>> +    for ( ; fsm < 6 && !feof (data); tok = fgetc (data)) {
>>          switch (tok) {
>>          case '\n':
>>              fsm = 1;
>> @@ -397,25 +150,58 @@
>>  static void
>>  check_compat_test (FILE* data, struct target_status* status)
>>  {
>> -    struct readback buf;
>> +    int read = 0;
>> +    unsigned fsm = 0;
>> +    char tok;
>>  
>>      assert (0 != data);
>>      assert (0 != status);
>>  
>> -    if (!rbinit (&buf, data)) {
>> -        status->status = ST_SYSTEM_ERROR;
>> -    } 
>> -    else if (rbbof (&buf)) {
>> +    tok = fgetc (data);
>> +
>> +    if (feof (data)) {
>>          /* target produced no output (regression test?) */
>>          status->status = ST_NO_OUTPUT;
>> +        return;
>>      }
>> -    else if (   !rbscanf (&buf, "## FailedAssertions = ", 
>> status->failed)
>> -             || !rbscanf (&buf, "## Assertions = ", status->assert)
>> -	     || !rbscanf (&buf, "## Warnings = ", status->t_warn)) {
>> +
>> +    for ( ; !feof (data); tok = fgetc (data)) {
>> +        switch (tok) {
>> +        case '\n':
>> +            fsm = 1;
>> +            break;
>> +        case '#':
>> +            if (1 == fsm || 2 == fsm)
>> +                ++fsm;
>> +            else
>> +                fsm = 0;
>> +            break;
>> +        case ' ':
>> +            if (3 == fsm) 
>> +                ++fsm;
>> +            else
>> +                fsm = 0;
>> +            break;
>> +        case 'W':
>> +            if (4 == fsm && !feof (data)) /* leading "## W" eaten */
>> +                read = fscanf (data, "arnings = %u\n## 
>> Assertions = %u\n"
>> +                       "## FailedAssertions = %u",
>> +                       &status->t_warn, &status->assert, 
>> &status->failed);
>> +        default:
>> +            fsm = 0;
>> +        }
>> +    }
>> +    if (3 != read) {
>>          status->status = ST_FORMAT;
>>      }
>>  }
>>  
>> +/**
>> +   Arbitrary constant controling static read buffer size.
>> +
>> +   @see check_example ()
>> +*/
>> +#define DELTA_BUF_LEN 64
>>  
>>  /**
>>     Parses output file out_name for the example target_name.
>> @@ -483,7 +269,7 @@
>>          }
>>      }
>>  
>> -    fref = fopen (ref_name, "rb");
>> +    fref = fopen (ref_name, "r");
>>  
>>      if (0 == fref) {
>>          int cache = errno; /* caching errno, as puts could 
>> alter it */
>> @@ -548,7 +334,7 @@
>>      assert (0 != options->argv [0]);
>>      out_name = output_name (options->argv [0]);
>>  
>> -    data = fopen (out_name, "rb");
>> +    data = fopen (out_name, "r");
>>  
>>      if (0 == data) {
>>          if (ENOENT != errno)
>>
>>
>>

RE: svn commit: r646002 - /stdcxx/trunk/util/output.cpp

Posted by Eric Lemings <Er...@roguewave.com>.
 
Does this mean we have no solution for resolving this issue (i.e.
back to Square One as it were)?  It's currently classified as one
of two Critical issues for 4.2.1 and Andrew's last comment is
"Further work is necessary".  Question is, how much work?

Thanks,
Brad.

> -----Original Message-----
> From: ablack@apache.org [mailto:ablack@apache.org] 
> Sent: Tuesday, April 08, 2008 11:13 AM
> To: commits@stdcxx.apache.org
> Subject: svn commit: r646002 - /stdcxx/trunk/util/output.cpp
> 
> Author: ablack
> Date: Tue Apr  8 10:12:34 2008
> New Revision: 646002
> 
> URL: http://svn.apache.org/viewvc?rev=646002&view=rev
> Log:
> 2008-04-08  Andrew Black  <ab...@roguewave.com>
> 
> 	STDCXX-426
> 	* util/output.cpp: Revert r643120, r644314, r645002, and
> 	r645445. The initial version of the patch for 
> STDCXX-426 (r643120)
> 	resulted in failed processing of test output on windows. The
> 	subsequent changes were efforts to resolve this failure, but
> 	resulted either in failed example diffing on windows, 
> or failed to
> 	resolve the observed misbehavior.
> 
> Modified:
>     stdcxx/trunk/util/output.cpp
> 
> Modified: stdcxx/trunk/util/output.cpp
> URL: 
> http://svn.apache.org/viewvc/stdcxx/trunk/util/output.cpp?rev=
> 646002&r1=646001&r2=646002&view=diff
> ==============================================================
> ================
> --- stdcxx/trunk/util/output.cpp (original)
> +++ stdcxx/trunk/util/output.cpp Tue Apr  8 10:12:34 2008
> @@ -44,239 +44,6 @@
>  #  define ENOENT 2
>  #endif   /* ENOENT */
>  
> -/**
> -   Arbitrary constant controling static read buffer size.
> -
> -   @see check_example ()
> -   @see rbread ()
> -*/
> -#define DELTA_BUF_LEN 64
> -
> -/** 
> -    This structure is used to encapsulate the data involved 
> in a backwards
> -    file read.
> -*/
> -struct readback {
> -    FILE* src;
> -    char buf[DELTA_BUF_LEN];
> -    long bpos;
> -};
> -
> -/**
> -   Initializes the provided readback structure, with the 
> provided file
> -   handle.
> -
> -   @param rb pointer to readback structure to initialize
> -   @param src file handle to initialize rb with.
> -   @returns true if successfully initialized, false otherwise.
> -*/
> -static bool
> -rbinit (struct readback* rb, FILE* const src)
> -{
> -    const size_t buflen = sizeof rb->buf;
> -    long fpos;
> -    assert (0 != rb);
> -    assert (0 != src);
> -
> -    rb->src = src;
> -    if (-1 == fseek (rb->src, 0, SEEK_END))
> -        return false;
> -    fpos = ftell (rb->src);
> -    if (-1 == fpos)
> -        return false;
> -
> -    if (fpos <= buflen)
> -        rb->bpos = fpos;
> -    else
> -        rb->bpos = buflen;
> -
> -    if (-1 == fseek (rb->src, fpos - rb->bpos, SEEK_SET))
> -        return false;
> -
> -    return rb->bpos == fread (rb->buf, 1, rb->bpos, rb->src);
> -}
> -
> -/** 
> -   This method is semi-analagous to feof.
> -   
> -   @param rb pointer to readback structure to check 
> begin-of-file state for
> -   @returns true if structure points to the begining of the 
> file, false 
> -   otherwise
> -*/
> -static bool
> -rbbof (const struct readback* const rb)
> -{
> -    assert (0 != rb);
> -    assert (0 != rb->src);
> -    return (0 == rb->bpos) && (0 == ftell (rb->src));
> -}
> -
> -/**
> -   Syncronizes the file handle underlying a readback structure to the
> -   pointer in the structure.  This method is called if you 
> wish to start
> -   reading forward from the current point in the readback structure.
> -
> -   @param rb pointer to the readback structure to syncronize.
> -   @return true if the structure was successfully syncronized, false 
> -   otherwise.
> -*/
> -static bool
> -rbsync (struct readback* const rb)
> -{
> -    assert (0 != rb);
> -    assert (0 != rb->src);
> -    const size_t buflen = sizeof rb->buf;
> -    long fpos = ftell (rb->src) - buflen;
> -    if (0 > fpos)
> -        fpos=0;
> -    fpos += rb->bpos;
> -    rb->bpos=0;
> -
> -    return -1 != fseek (rb->src, fpos, SEEK_SET);
> -}
> -
> -/**
> -   Updates the provided readback structure, returning the 
> last unread 
> -   character in the file.  This method is semi-analagous to fgetc.
> -   
> -   @param rb pointer to readback structure to read from.
> -   @return EOF if beginning of file or I/O error, read character
> -   otherwise.
> -*/
> -static char
> -rbgetc (struct readback* rb)
> -{
> -    const size_t buflen = sizeof rb->buf;
> -    assert (0 != rb);
> -    assert (0 != rb->src);
> -    if (!rb->bpos) {
> -        const size_t bufdelta = buflen << 1;
> -        long fpos = ftell (rb->src);
> -        long seek;
> -
> -        if (-1 == fpos)
> -            return EOF;
> -
> -        if (bufdelta <= fpos) {
> -            seek = fpos - bufdelta;
> -            rb->bpos = buflen;
> -        }
> -        else if (buflen < fpos) {
> -            seek = 0;
> -            rb->bpos = fpos - buflen;
> -        }
> -        else {
> -            fseek (rb->src, 0, SEEK_SET);
> -            return EOF;
> -        }
> -
> -        if (-1 == fseek (rb->src, seek, SEEK_SET))
> -            return EOF;
> -
> -        if (rb->bpos != fread (rb->buf, 1, rb->bpos, rb->src))
> -            return EOF;
> -    }
> -    assert (0 < rb->bpos && rb->bpos <= buflen);
> -
> -    return rb->buf [--rb->bpos];
> -}
> -
> -/**
> -   This method is semi-analagous to fscanf, with some key 
> differences.
> -   First, it opperates on a readback structure.  Second, it 
> scans until
> -   either the search pattern is matched, or until the 
> begining of the file
> -   is reached.  Third, it reads backwards from the end of the file.
> -
> -   Limitations: string to match can't end in a repeated set 
> of characters
> -   (ie: 'singing' has the repeated characters 'ing')
> -
> -   @param rb pointer to readback structure to operate on
> -   @param match string to search for
> -   @return true if match is found, false if begining of file 
> is reached
> -*/
> -static bool
> -rbscanf (struct readback* rb, const char* const match)
> -{
> -    char tok;
> -    size_t matched;
> -    size_t count;
> -    assert (0 != rb);
> -    assert (0 != match);
> -
> -    matched = count = strlen (match) - 1;
> -    
> -    for(tok = rbgetc(rb); matched && !rbbof (rb); tok = 
> rbgetc (rb)) {
> -        if (tok == match [matched])
> -            --matched;
> -        else
> -            matched = count;
> -    }
> -    return !matched;
> -}
> -
> -/**
> -   This method is semi-analagous to fscanf, reading 
> backwards on a readback
> -   structure, starting from the end of the file.
> -
> -   This method searches backwards for the first (last) 
> number and captures
> -   it.  It then checks if the match string exists directly 
> after (before)
> -   the captured value.  If this is the case, a match is 
> considered to have
> -   been found, and the captured value is copied to val.  
> Otherwise, the 
> -   search process is restarted.  If the search hits the 
> beginning of the 
> -   file, matching is considered to have failed, and val is unaltered.
> -
> -   Limitations: string to match can't end in a repeated set 
> of characters
> -   (ie: 'singing' has the repeated characters 'ing'.) or a 
> repeated set
> -   of characters, bordering on either side of one or more 
> numbers (ie: 
> -   'xyzzy9zzy' has the repeated characters 'zzy')
> -
> -   @param rb pointer to readback structure to operate on.
> -   @param match string to search for.
> -   @param val reference to variable to store captured value in.
> -   @return true if match is found, false if begining of file 
> is reached.
> -*/
> -static bool
> -rbscanf (struct readback* rb, const char* const match, unsigned& val)
> -{
> -    size_t count;
> -    unsigned tval =0;
> -    unsigned radix;
> -    assert (0 != rb);
> -    assert (0 != match);
> -
> -    count = strlen (match) - 1;
> -    
> -    while (!rbbof(rb)) {
> -        char tok;
> -        size_t matched = count;
> -
> -        tval=0;
> -        radix=1;
> -
> -        /* Search for a numeric digit */
> -        for (tok = rbgetc (rb); 
> -            (tok < '0' || tok > '9') && !rbbof (rb); 
> -             tok = rbgetc (rb)) /* Do nothing */;
> -
> -        /* Read in the number. */
> -        for ( ; tok >= '0' && tok <= '9' && !rbbof (rb); 
> -            tok = rbgetc (rb)) {
> -            tval += (tok-'0') * radix;
> -            radix *= 10;
> -        }
> -
> -        /* Make certain the content prior to the number in the file 
> -           matches the search pattern. */
> -        for( ; matched && tok == match [matched] && !rbbof (rb); 
> -            tok = rbgetc (rb), --matched) /* Do nothing */;
> -
> -        if (tok == match [matched]) {
> -            val = tval;
> -            return true;
> -        }
> -    }
> -    return false;
> -}
>  
>  /**
>     Parses contents of the open file handle data for test target_name.
> @@ -291,7 +58,6 @@
>  static void
>  check_test (FILE* data, struct target_status* status)
>  {
> -    struct readback buf;
>      unsigned r_lvl    = 0;   /* diagnostic severity level */
>      unsigned r_active = 0;   /* number of active diagnostics */
>      unsigned r_total  = 0;   /* total number of diagnostics */
> @@ -307,28 +73,15 @@
>      assert (0 != data);
>      assert (0 != status);
>  
> -    if (!rbinit (&buf, data)) {
> -        status->status = ST_SYSTEM_ERROR;
> -        return;
> -    }
> +    tok = fgetc (data);
>  
> -    if (rbbof (&buf)) {
> +    if (feof (data)) {
>          /* target produced no output (regression test?) */
>          status->status = ST_NO_OUTPUT;
>          return;
>      }
>  
> -    if (!rbscanf (&buf, "| INACTIVE |")) {
> -        status->status = ST_FORMAT;
> -        return;
> -    }
> -
> -    rbsync(&buf);
> -
> -    /* While it'd probably be (slightly) faster to seek to a 
> fixed position 
> -       after the file pointer has been synced, this should 
> be more reliable 
> -    */
> -    for (tok = fgetc (data); fsm < 6 && !feof (data); tok = 
> fgetc (data)) {
> +    for ( ; fsm < 6 && !feof (data); tok = fgetc (data)) {
>          switch (tok) {
>          case '\n':
>              fsm = 1;
> @@ -397,25 +150,58 @@
>  static void
>  check_compat_test (FILE* data, struct target_status* status)
>  {
> -    struct readback buf;
> +    int read = 0;
> +    unsigned fsm = 0;
> +    char tok;
>  
>      assert (0 != data);
>      assert (0 != status);
>  
> -    if (!rbinit (&buf, data)) {
> -        status->status = ST_SYSTEM_ERROR;
> -    } 
> -    else if (rbbof (&buf)) {
> +    tok = fgetc (data);
> +
> +    if (feof (data)) {
>          /* target produced no output (regression test?) */
>          status->status = ST_NO_OUTPUT;
> +        return;
>      }
> -    else if (   !rbscanf (&buf, "## FailedAssertions = ", 
> status->failed)
> -             || !rbscanf (&buf, "## Assertions = ", status->assert)
> -	     || !rbscanf (&buf, "## Warnings = ", status->t_warn)) {
> +
> +    for ( ; !feof (data); tok = fgetc (data)) {
> +        switch (tok) {
> +        case '\n':
> +            fsm = 1;
> +            break;
> +        case '#':
> +            if (1 == fsm || 2 == fsm)
> +                ++fsm;
> +            else
> +                fsm = 0;
> +            break;
> +        case ' ':
> +            if (3 == fsm) 
> +                ++fsm;
> +            else
> +                fsm = 0;
> +            break;
> +        case 'W':
> +            if (4 == fsm && !feof (data)) /* leading "## W" eaten */
> +                read = fscanf (data, "arnings = %u\n## 
> Assertions = %u\n"
> +                       "## FailedAssertions = %u",
> +                       &status->t_warn, &status->assert, 
> &status->failed);
> +        default:
> +            fsm = 0;
> +        }
> +    }
> +    if (3 != read) {
>          status->status = ST_FORMAT;
>      }
>  }
>  
> +/**
> +   Arbitrary constant controling static read buffer size.
> +
> +   @see check_example ()
> +*/
> +#define DELTA_BUF_LEN 64
>  
>  /**
>     Parses output file out_name for the example target_name.
> @@ -483,7 +269,7 @@
>          }
>      }
>  
> -    fref = fopen (ref_name, "rb");
> +    fref = fopen (ref_name, "r");
>  
>      if (0 == fref) {
>          int cache = errno; /* caching errno, as puts could 
> alter it */
> @@ -548,7 +334,7 @@
>      assert (0 != options->argv [0]);
>      out_name = output_name (options->argv [0]);
>  
> -    data = fopen (out_name, "rb");
> +    data = fopen (out_name, "r");
>  
>      if (0 == data) {
>          if (ENOENT != errno)
> 
> 
>