Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 51277 invoked by uid 500); 29 Dec 2002 20:04:43 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 51157 invoked from network); 29 Dec 2002 20:04:41 -0000 Date: Sun, 29 Dec 2002 12:20:01 -0800 (PST) From: X-X-Sender: To: cc: Subject: Re: cvs commit: apr/test testfile.c In-Reply-To: <20021229195134.89020.qmail@icarus.apache.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N > -#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