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:20:01 GMT

>   -#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