Return-Path: Delivered-To: apmail-apr-cvs-archive@apr.apache.org Received: (qmail 39517 invoked by uid 500); 15 Jun 2001 20:04:50 -0000 Mailing-List: contact cvs-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Reply-To: dev@apr.apache.org Delivered-To: mailing list cvs@apr.apache.org Received: (qmail 39405 invoked by uid 1211); 15 Jun 2001 20:04:45 -0000 Date: 15 Jun 2001 20:04:45 -0000 Message-ID: <20010615200445.39404.qmail@apache.org> From: sussman@apache.org To: apr-cvs@apache.org Subject: cvs commit: apr/file_io/unix dir.c sussman 01/06/15 13:04:44 Modified: test Makefile.in file_io/unix dir.c Added: test testdir.c Log: Bugfix for the unix version of apr_dir_read(). I caught it on my FreeBSD 4.3 machine while trying to debug Subversion. :) It happened in the thread-less section of code (since threads are turned off on FreeBSD). It turns out that apr_dir_remove() had previously failed and set the global `errno'. Then later on I tried to do a looping apr_dir_read() in an empty directory. After returning `.' and `..' as usual, the underlying unix readdir() returned NULL -- as it should. However, apr_dir_read() didn't return ENOENT as it ought to; instead, it noticed that `errno' still had an old value and returned that instead. The solution is to zero errno beforehand, unless someone has a better suggestion. * file_io/unix/dir.c (apr_dir_read): clear errno *before* you depend on its value after calling readdir(). * test/testdir.c: new regression test for this bug. * test/Makefile.in (testdir): build the new test. Revision Changes Path 1.57 +4 -0 apr/test/Makefile.in Index: Makefile.in =================================================================== RCS file: /home/cvs/apr/test/Makefile.in,v retrieving revision 1.56 retrieving revision 1.57 diff -u -r1.56 -r1.57 --- Makefile.in 2001/05/31 03:30:05 1.56 +++ Makefile.in 2001/06/15 20:04:39 1.57 @@ -4,6 +4,7 @@ sendfile@EXEEXT@ \ server@EXEEXT@ \ testfile@EXEEXT@ \ + testdir@EXEEXT@ \ testnames@EXEEXT@ \ testflock@EXEEXT@ \ testproc@EXEEXT@ \ @@ -42,6 +43,9 @@ testfile@EXEEXT@: testfile.lo $(LOCAL_LIBS) $(LINK) testfile.lo $(LOCAL_LIBS) $(ALL_LIBS) + +testdir@EXEEXT@: testdir.lo $(LOCAL_LIBS) + $(LINK) testdir.lo $(LOCAL_LIBS) $(ALL_LIBS) testnames@EXEEXT@: testnames.lo $(LOCAL_LIBS) $(LINK) testnames.lo $(LOCAL_LIBS) $(ALL_LIBS) 1.1 apr/test/testdir.c Index: testdir.c =================================================================== /* ==================================================================== * The Apache Software License, Version 1.1 * * Copyright (c) 2000-2001 The Apache Software Foundation. All rights * reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * * 1. Redistributions of source code must retain the above copyright * notice, this list of conditions and the following disclaimer. * * 2. Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. * * 3. The end-user documentation included with the redistribution, * if any, must include the following acknowledgment: * "This product includes software developed by the * Apache Software Foundation (http://www.apache.org/)." * Alternately, this acknowledgment may appear in the software itself, * if and wherever such third-party acknowledgments normally appear. * * 4. The names "Apache" and "Apache Software Foundation" must * not be used to endorse or promote products derived from this * software without prior written permission. For written * permission, please contact apache@apache.org. * * 5. Products derived from this software may not be called "Apache", * nor may "Apache" appear in their name, without prior written * permission of the Apache Software Foundation. * * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * ==================================================================== * * This software consists of voluntary contributions made by many * individuals on behalf of the Apache Software Foundation. For more * information on the Apache Software Foundation, please see * . */ #include #include #include #include #include "apr_file_io.h" #include "apr_file_info.h" #include "apr_errno.h" #include "apr_general.h" #include "apr_lib.h" #include "test_apr.h" /* Test for a (fixed) bug in apr_dir_read(). This bug only happened in threadless cases. */ int main(void) { apr_pool_t *pool; apr_file_t *thefile = NULL; apr_finfo_t finfo; apr_int32_t finfo_flags = APR_FINFO_TYPE | APR_FINFO_NAME; apr_dir_t *this_dir; printf("APR Directory Read Test\n===========================\n\n"); STD_TEST_NEQ("Initializing APR", apr_initialize()) atexit(apr_terminate); STD_TEST_NEQ("Creating the main pool we'll use", apr_pool_create(&pool, NULL)) fprintf(stdout, "Testing for readdir() bug.\n"); /* Make two empty directories, and put a file in one of them. */ STD_TEST_NEQ(" Creating empty dir1", apr_dir_make("dir1", APR_OS_DEFAULT, pool)) STD_TEST_NEQ(" Creating empty dir2", apr_dir_make("dir2", APR_OS_DEFAULT, pool)) STD_TEST_NEQ(" Creating dir1/file1", apr_file_open(&thefile, "dir1/file1", APR_READ | APR_WRITE | APR_CREATE, APR_OS_DEFAULT, pool)) /* Try to remove dir1. This should fail because it's not empty. However, on a platform with threads disabled (such as FreeBSD), `errno' will be set as a result. */ TEST_EQ(" Failing to remove dir1", apr_dir_remove("dir1", pool), APR_SUCCESS, "OK", "Failed") /* Read `.' and `..' out of dir2. */ STD_TEST_NEQ(" Opening dir2", apr_dir_open(&this_dir, "dir2", pool)) STD_TEST_NEQ(" reading `.' entry", apr_dir_read(&finfo, finfo_flags, this_dir)) STD_TEST_NEQ(" reading `..' entry", apr_dir_read(&finfo, finfo_flags, this_dir)) /* Now, when we attempt to do a third read of empty dir2, and the underlying system readdir() returns NULL, the old value of errno shouldn't cause a false alarm. We should get an ENOENT back from apr_dir_read, and *not* the old errno. */ TEST_NEQ(" get ENOENT on 3rd read", apr_dir_read(&finfo, finfo_flags, this_dir), APR_ENOENT, "OK", "Failed") /* Cleanup */ STD_TEST_NEQ(" Cleanup file1", apr_file_remove("dir1/file1", pool)) STD_TEST_NEQ(" Cleanup dir1", apr_dir_remove("dir1", pool)) STD_TEST_NEQ(" Cleanup dir2", apr_dir_remove("dir2", pool)) apr_pool_destroy(pool); printf("\nAll tests passed OK\n"); return 1; } 1.56 +4 -0 apr/file_io/unix/dir.c Index: dir.c =================================================================== RCS file: /home/cvs/apr/file_io/unix/dir.c,v retrieving revision 1.55 retrieving revision 1.56 diff -u -r1.55 -r1.56 --- dir.c 2001/02/25 20:39:29 1.55 +++ dir.c 2001/06/15 20:04:43 1.56 @@ -123,6 +123,10 @@ if(!ret && thedir->entry != retent) ret = APR_ENOENT; #else + /* We're about to call a non-thread-safe readdir() that may + possibly set `errno', and the logic below actually cares about + errno after the call. Therefore we need to clear errno first. */ + errno = 0; thedir->entry = readdir(thedir->dirstruct); if (thedir->entry == NULL) { /* If NULL was returned, this can NEVER be a success. Can it?! */