apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@apache.org>
Subject Re: cvs commit: apr/test testfile.c
Date Sun, 29 Dec 2002 20:25:41 GMT

Sorry about this.  I just re-read the patch and saw that the comment was
removed but the test was kept.  Ignore this message please.

Ryan

On Sun, 29 Dec 2002 rbb@apache.org wrote:

>
> >   -#if 0
> >   -    /* I consider this a bug, if we are going to return an error, we shouldn't
> >   -     * allocate the file pointer.  But, this would make us fail the text, so
> >   -     * I am commenting it out for now.
> >   -     */
> >        CuAssertPtrEquals(tc, NULL, thefile);
> >   -#endif
> >   -    apr_file_close(thefile);
> >   +    if (thefile) {
> >   +        apr_file_close(thefile);
> >   +    }
>
> Ummmm.....  You just removed a perfectly valid complaint and test.  If you
> want to have a conversation about it cool, but please don't just remove a
> comment that is pointing out a potential bug.
>
>
> >   -#if 0
> >   -    /* I consider this a bug, if we are going to return an error, we shouldn't
> >   -     * allocate the file pointer.  But, this would make us fail the text, so
> >   -     * I am commenting it out for now.
> >   -     */
> >        CuAssertPtrEquals(tc, NULL, thefile);
> >   -#endif
> >   -    /* And this too is a bug... Win32 (correctly) does not allocate
> >   -     * an apr_file_t, and (correctly) returns NULL.  Closing objects
> >   -     * that failed to open is invalid.  Apparently someone is doing so.
> >   -     */
>
> Will, please stop doing stuff like this.  The whole point of the test
> suite is to catch bugs.  Your changes aren't doing that.  You are working
> around problems in the test suite, and letting bugs slip through on
> non-windows platforms.
>
> The comment above tells me that Windows and Unix are out of sync.  It
> looks like the CuTestPtrEquals(tc, NULL, ...) should be added to both of
> these tests, and Unix should be fixed to correctly not allocate the
> pointer if the file_open fails.
>
> Ryan
>
>
>


Mime
View raw message