You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2019/07/02 15:58:19 UTC

svn commit: r1862435 - in /apr/apr/trunk: CHANGES file_io/os2/dir.c file_io/win32/dir.c include/apr_file_info.h test/testdir.c

Author: jorton
Date: Tue Jul  2 15:58:19 2019
New Revision: 1862435

URL: http://svn.apache.org/viewvc?rev=1862435&view=rev
Log:
* include/apr_file_info.h: Clarify pool handling for apr_dir_read and
  apr_dir_pread.

* file_io/win32/dir.c, file_io/os2/dir.c (apr_dir_read): Duplicate the
  returned filename so the call has no side-effects on apr_finfo_t
  structures passed to previous invocations of the function.

* test/testdir.c (test_read_side_effects): Add test case for
  side-effects of apr_dir_read.

Modified:
    apr/apr/trunk/CHANGES
    apr/apr/trunk/file_io/os2/dir.c
    apr/apr/trunk/file_io/win32/dir.c
    apr/apr/trunk/include/apr_file_info.h
    apr/apr/trunk/test/testdir.c

Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1862435&r1=1862434&r2=1862435&view=diff
==============================================================================
--- apr/apr/trunk/CHANGES [utf-8] (original)
+++ apr/apr/trunk/CHANGES [utf-8] Tue Jul  2 15:58:19 2019
@@ -1,6 +1,9 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 2.0.0
 
+  *) apr_dir_read(): The returned finfo->name field is now duplicated
+     into the pool for all implementations.  [Joe Orton]
+
   *) apr_dir_pread(): Add function which restricts per-read memory
      consumption to a different pool to the apr_dir_t object.
      [Joe Orton]

Modified: apr/apr/trunk/file_io/os2/dir.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/os2/dir.c?rev=1862435&r1=1862434&r2=1862435&view=diff
==============================================================================
--- apr/apr/trunk/file_io/os2/dir.c (original)
+++ apr/apr/trunk/file_io/os2/dir.c Tue Jul  2 15:58:19 2019
@@ -122,7 +122,7 @@ APR_DECLARE(apr_status_t) apr_dir_pread(
         apr_os2_time_to_apr_time(&finfo->ctime, thedir->entry.fdateCreation,
                                  thedir->entry.ftimeCreation);
 
-        finfo->name = thedir->entry.achName;
+        finfo->name = apr_pstrdup(pool, thedir->entry.achName);
         finfo->valid = APR_FINFO_NAME | APR_FINFO_MTIME | APR_FINFO_ATIME |
             APR_FINFO_CTIME | APR_FINFO_TYPE | APR_FINFO_SIZE |
             APR_FINFO_CSIZE;

Modified: apr/apr/trunk/file_io/win32/dir.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/dir.c?rev=1862435&r1=1862434&r2=1862435&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/dir.c (original)
+++ apr/apr/trunk/file_io/win32/dir.c Tue Jul  2 15:58:19 2019
@@ -152,7 +152,7 @@ APR_DECLARE(apr_status_t) apr_dir_pread(
     if ((rv = unicode_to_utf8_path(thedir->name, APR_FILE_MAX * 3 + 1, 
                                    thedir->entry->cFileName)))
         return rv;
-    fname = thedir->name;
+    fname = apr_pstrdup(pool, thedir->name);
 
     fillin_fileinfo(finfo, (WIN32_FILE_ATTRIBUTE_DATA *) thedir->entry,
                     0, 1, fname, wanted);

Modified: apr/apr/trunk/include/apr_file_info.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_file_info.h?rev=1862435&r1=1862434&r2=1862435&view=diff
==============================================================================
--- apr/apr/trunk/include/apr_file_info.h (original)
+++ apr/apr/trunk/include/apr_file_info.h Tue Jul  2 15:58:19 2019
@@ -257,15 +257,18 @@ APR_DECLARE(apr_status_t) apr_dir_close(
  * @param wanted The desired apr_finfo_t fields, as a bit flag of APR_FINFO_
                  values 
  * @param thedir the directory descriptor returned from apr_dir_open
+ *
  * @remark No ordering is guaranteed for the entries read.
+ *         @c finfo->pool is set to the pool used to create @a thedir,
+ *         and @c finfo->name is allocated from that pool.
  *
  * @note If @c APR_INCOMPLETE is returned all the fields in @a finfo may
  *       not be filled in, and you need to check the @c finfo->valid bitmask
  *       to verify that what you're looking for is there. When no more
  *       entries are available, APR_ENOENT is returned.
  *
- * @warning Memory will be allocated in the pool passed to apr_dir_open;
- *          use apr_dir_pread() and a temporary pool to restrict memory
+ * @warning Allocations will use the directory pool; use
+ *          apr_dir_pread() and a temporary pool to restrict memory
  *          consumption for a large directory.
  */                        
 APR_DECLARE(apr_status_t) apr_dir_read(apr_finfo_t *finfo, apr_int32_t wanted,
@@ -278,7 +281,10 @@ APR_DECLARE(apr_status_t) apr_dir_read(a
                  values 
  * @param thedir the directory descriptor returned from apr_dir_open
  * @param pool the pool to use for allocations
- * @remark No ordering is guaranteed for the entries read.
+
+ * @remark No ordering is guaranteed for the entries read.  
+ * @remark @c finfo->pool is set to @a pool, and @c finfo->name is
+ *         allocated from that pool.
  *
  * @note If @c APR_INCOMPLETE is returned all the fields in @a finfo may
  *       not be filled in, and you need to check the @c finfo->valid bitmask

Modified: apr/apr/trunk/test/testdir.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/test/testdir.c?rev=1862435&r1=1862434&r2=1862435&view=diff
==============================================================================
--- apr/apr/trunk/test/testdir.c (original)
+++ apr/apr/trunk/test/testdir.c Tue Jul  2 15:58:19 2019
@@ -21,6 +21,7 @@
 #include "apr_file_info.h"
 #include "apr_errno.h"
 #include "apr_general.h"
+#include "apr_strings.h"
 #include "apr_lib.h"
 #include "apr_thread_proc.h"
 #include "testutil.h"
@@ -460,6 +461,36 @@ static void test_pread(abts_case *tc, vo
 }
 #endif
 
+/* Ensure that apr_dir_read() doesn't have side-effects, because
+ * finfo->name points to a static buffer inside the apr_dir_t */
+static void test_read_side_effects(abts_case *tc, void *data)
+{
+    apr_dir_t *dir;
+    apr_finfo_t f1;
+    apr_finfo_t f2;
+    char name[APR_PATH_MAX], fname[APR_PATH_MAX];
+
+    APR_ASSERT_SUCCESS(tc, "apr_dir_open failed", apr_dir_open(&dir, "data", p));
+    
+    APR_ASSERT_SUCCESS(tc, "apr_dir_read failed",
+                       apr_dir_read(&f1, APR_FINFO_DIRENT, dir));
+
+    if (f1.name)
+        apr_cpystrn(name, f1.name, sizeof name);
+    if (f1.fname)
+        apr_cpystrn(fname, f1.fname, sizeof fname);
+
+    APR_ASSERT_SUCCESS(tc, "second apr_dir_read failed",
+                       apr_dir_read(&f2, APR_FINFO_DIRENT, dir));
+
+    if (f1.name)
+        ABTS_STR_EQUAL(tc, name, f1.name);
+    if (f1.fname)
+        ABTS_STR_EQUAL(tc, fname, f1.fname);
+    
+    APR_ASSERT_SUCCESS(tc, "apr_dir_close failed", apr_dir_close(dir));
+}
+
 abts_suite *testdir(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -484,6 +515,7 @@ abts_suite *testdir(abts_suite *suite)
 #if APR_POOL_DEBUG
     abts_run_test(suite, test_pread, NULL);
 #endif
+    abts_run_test(suite, test_read_side_effects, NULL);
     
     return suite;
 }