You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Lucian Adrian Grijincu <lu...@gmail.com> on 2007/11/06 02:08:09 UTC
Re: svn commit: r592216 - in /apr/apr/branches/1.2.x: include/apr_file_io.h test/testdir.c
On Nov 6, 2007 2:51 AM, <wr...@apache.org> wrote:
> Author: wrowe
> Date: Mon Nov 5 16:51:20 2007
> New Revision: 592216
>
> URL: http://svn.apache.org/viewvc?rev=592216&view=rev
> Log:
> It is entirely pointless to have nonportable behaviors as examples
> to end users of the library. Good point, however, for an @tip.
>
> Backports: 592215
>
> Modified:
> apr/apr/branches/1.2.x/include/apr_file_io.h
> apr/apr/branches/1.2.x/test/testdir.c
>
> Modified: apr/apr/branches/1.2.x/include/apr_file_io.h
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/apr_file_io.h?rev=592216&r1=592215&r2=592216&view=diff
> ==============================================================================
> --- apr/apr/branches/1.2.x/include/apr_file_io.h (original)
> +++ apr/apr/branches/1.2.x/include/apr_file_io.h Mon Nov 5 16:51:20 2007
> @@ -724,6 +724,8 @@
> * Remove directory from the file system.
> * @param path the path for the directory to be removed. (use / on all systems)
> * @param pool the pool to use.
> + * @tip removing a directory which is in-use (e.g., the current working
> + * directory, or during apr_dir_read, or with an open file) is not portable.
> */
> APR_DECLARE(apr_status_t) apr_dir_remove(const char *path, apr_pool_t *pool);
>
>
> Modified: apr/apr/branches/1.2.x/test/testdir.c
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/test/testdir.c?rev=592216&r1=592215&r2=592216&view=diff
> ==============================================================================
> --- apr/apr/branches/1.2.x/test/testdir.c (original)
> +++ apr/apr/branches/1.2.x/test/testdir.c Mon Nov 5 16:51:20 2007
> @@ -234,20 +234,9 @@
>
> APR_ASSERT_SUCCESS(tc, "change to temp dir", apr_filepath_set(path, p));
>
> - rv = apr_dir_remove(path, p);
> - /* Some platforms cannot remove a directory which is in use. */
> - if (rv == APR_SUCCESS) {
> - ABTS_ASSERT(tc, "fail to create dir",
> - apr_dir_make_recursive("foobar", APR_OS_DEFAULT,
> - p) != APR_SUCCESS);
> - }
> -
> APR_ASSERT_SUCCESS(tc, "restore cwd", apr_filepath_set(cwd, p));
>
> - if (rv) {
> - apr_dir_remove(path, p);
> - ABTS_NOT_IMPL(tc, "cannot remove in-use directory");
> - }
> + APR_ASSERT_SUCCESS(tc, "remove cwd", rv = apr_dir_remove(path, p));
What purpose does "rv = " have in this code?
the apr_status_t isn't used anywhere in there
[[[To put it in context:
static void test_rmkdir_nocwd(abts_case *tc, void *data)
{
char *cwd, *path;
apr_status_t rv;
APR_ASSERT_SUCCESS(tc, "make temp dir",
apr_dir_make("dir3", APR_OS_DEFAULT, p));
APR_ASSERT_SUCCESS(tc, "obtain cwd", apr_filepath_get(&cwd, 0, p));
APR_ASSERT_SUCCESS(tc, "determine path to temp dir",
apr_filepath_merge(&path, cwd, "dir3", 0, p));
APR_ASSERT_SUCCESS(tc, "change to temp dir", apr_filepath_set(path, p));
APR_ASSERT_SUCCESS(tc, "restore cwd", apr_filepath_set(cwd, p));
APR_ASSERT_SUCCESS(tc, "remove cwd", rv = apr_dir_remove(path, p));
}
]]]
> }
>
>
>
>
>
--
Lucian
Re: svn commit: r592216 - in /apr/apr/branches/1.2.x:
include/apr_file_io.h test/testdir.c
Posted by Bojan Smojver <bo...@rexursive.com>.
On Tue, 2007-11-06 at 03:08 +0200, Lucian Adrian Grijincu wrote:
> What purpose does "rv = " have in this code?
> the apr_status_t isn't used anywhere in there
A leftover-cut-n-paste purpose? :-)
--
Bojan
Re: svn commit: r592216 - in /apr/apr/branches/1.2.x: include/apr_file_io.h
test/testdir.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Lucian Adrian Grijincu wrote:
> On Nov 6, 2007 2:51 AM, <wr...@apache.org> wrote:
>> + APR_ASSERT_SUCCESS(tc, "remove cwd", rv = apr_dir_remove(path, p));
>
> What purpose does "rv = " have in this code?
> the apr_status_t isn't used anywhere in there
None - good catch, thanks. Patching now.
Bill