apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] apr_dir_remove_recursively
Date Tue, 22 May 2001 22:41:31 GMT
On Tue, May 22, 2001 at 08:44:19AM -0500, Ben Collins-Sussman wrote:
>...
> This routine currently lives in a Subversion library.  I'd like to
> move it into apr_file_io.h, but I'm not sure where the actual code
> should live.  It's implemented using nothing but other apr file
> routines;  does this mean duplicating the code into file_io/unix,
> file_io/win32, file_io/os2?

Nope. Put the file into file_io/unix/something.c. The other platforms will
just use/refer to it. See unix/fullrw.c and os2/fullrw.c for an example.
(you'll also see that apr/libapr.dsp refers to files in the unix/ dirs)

> apr_status_t
> apr_dir_remove_recursively (const char *path, apr_pool_t *pool)
> {
>   apr_status_t status;
>   apr_dir_t *this_dir;
>   apr_finfo_t this_entry;
>   apr_pool_t *subpool;
>   apr_int32_t flags = APR_FINFO_TYPE | APR_FINFO_NAME;
> 
>   status = apr_pool_create (&subpool, pool);
>   if (! (APR_STATUS_IS_SUCCESS (status))) return status;
> 
>   status = apr_dir_open (&this_dir, path, subpool);
>   if (! (APR_STATUS_IS_SUCCESS (status))) return status;
> 
>   for (status = apr_dir_read (&this_entry, flags, this_dir);
>        APR_STATUS_IS_SUCCESS (status);
>        status = apr_dir_read (&this_entry, flags, this_dir))
>     {
>       char *fullpath = apr_pstrcat (subpool, path, "/", this_entry.name, NULL);
> 
>       if (this_entry.filetype == APR_DIR)
>         {
>           if ((strcmp (this_entry.name, ".") == 0)
>               || (strcmp (this_entry.name, "..") == 0))
>             continue;
> 
>           status = apr_dir_remove_recursively (fullpath, subpool);
>           if (! (APR_STATUS_IS_SUCCESS (status))) return status;
>         }
>       else if (this_entry.filetype == APR_REG)
>         {
>           status = apr_file_remove (fullpath, subpool);
>           if (! (APR_STATUS_IS_SUCCESS (status))) return status;
>         }
>     }
> 
>   if (! (APR_STATUS_IS_ENOENT (status)))
>     return status;
> 
>   else
>     {
>       status = apr_dir_close (this_dir);
>       if (! (APR_STATUS_IS_SUCCESS (status))) return status;
>     }
> 
>   status = apr_dir_remove (path, subpool);
>   if (! (APR_STATUS_IS_SUCCESS (status))) return status;
> 
>   apr_pool_destroy (subpool);
> 
>   return APR_SUCCESS;
> }

This should be in the Apache coding style before committing. Notably: no
space between symbol and opening paren, braces on the if/else lines, and the
return status lines on a separate line.

Two code comments:

1) APR_STATUS_IS_SUCCESS is kind of a bogus macro; by definition,
   APR_SUCCESS is zero. Testing it can be done more clearly than using the
   macro.

2) the strcmp() calls can be tossed:

    if (this_entry.name[0] == '.'
        && (this_entry.name[1] == '\0'
	    || (this_entry.name[1] == '.' && this_entry.name[2] == '\0')))
        continue;


Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message