Hi,
You're right, that was quite a memory hog.
I've rewritten it and this looks quite OK to me.

    * make 2 pools: one to hold the stack directories and one for temp data
    * create the stack and push the passed-in directory parameter
    * while (!is_stack_empty)
          o clear the temp pool
          o find the full path to the current directory (concat all the
            entries in the stack)
          o loop through the files in the current dir
                + if a file is a directory, push it to the stack and
                  break out of the inner loop
                + delete all the rest of the files
          o if we didn't break out of the loop, we must have deleted
            everything inside the directory, so delete this directory
            and pop it from the stack
    * cleanup pools

Joe Orton wrote:
> On Thu, Dec 14, 2006 at 03:57:18PM +0200, Lucian Adrian Grijincu wrote:
>   
>> I've modified the version posted earlier(2001) by Ben Collins-Sussman in
>> this way:
>>     
>
> Hi Lucian, thanks for sending the patch in.
>
> The way that's written has pretty bad memory consumption behaviour.  
> Using stack recursion to iterate down the directory tree is not really 
> going to fly; it is liable to cause a stack overflow and crash for a 
> really deep directory tree.  Eating pool memory proportional to breadth 
> *and* depth of the tree is not really great either.  Ideally this should 
> work something like:
>
> 1. allocate a stack as char * array
> 2. push passed-in directory name onto stack
> 3. while(stack-not-empty):
>  a) create subpool
>   
    it's more efficient to clear a loop_pool ...
>  b) pop top directory off stack
>  c) open directory and loop through entries:
>   i) for a directory, pop it on the stack
>   ii) for anything else, delete it
>  d) delete subpool
>
> (not sure whether processing the tree in FILO order or FIFO order is 
> better, I'd guess FIFO)
>
>   
when we find a subdir we stop reading the current dir and recursively
delete the subdir. when we finish deleting the subdir, we start reading
the former dir again. FIFO.
> joe
>   


-- 
Best regards,

Lucian Adrian Grijincu
Software Developer
Avira Soft srl

[EMAIL PROTECTED]
http://www.avira.com

#define APR_DIR_REMOVE_RECURSIVELY_ARRSIZE (6) /*what would be a propper value?*/
#define PATH_SEPARATOR '/' /*is one defined in a header somewhere else ?*/

apr_status_t apr_dir_remove_recursively(const char *param_path, apr_pool_t *pool)
{
    apr_status_t status, retcode, dirstatus;
    apr_dir_t *this_dir;
    apr_finfo_t this_entry;
    apr_pool_t *array_pool, *loop_pool;
    apr_array_header_t *arr;
    apr_int32_t flags = APR_FINFO_TYPE | APR_FINFO_NAME;
    char *dir_path, *file_path;
    int dir_is_empty;

    retcode = APR_SUCCESS;
    status = apr_pool_create (&array_pool, pool);
    if (status != APR_SUCCESS)  {
        return status;
    }
    status = apr_pool_create (&loop_pool, pool);
    if (status != APR_SUCCESS)  {
        apr_pool_destroy (array_pool);
        return status;
    }


    arr = apr_array_make(array_pool, APR_DIR_REMOVE_RECURSIVELY_ARRSIZE, sizeof(const char*));
    if (arr == NULL) {
        apr_pool_destroy (array_pool);
        apr_pool_destroy (loop_pool);        
        return APR_ENOMEM;
    }    
    *(const char**)apr_array_push (arr) = param_path;



    while ( ! apr_is_empty_array (arr) ) {
        apr_pool_clear (loop_pool);
        dir_path = apr_array_pstrcat (loop_pool, arr, PATH_SEPARATOR);
        if (dir_path == NULL) {
            retcode = APR_ENOMEM;
            break;
        }
        
        dir_is_empty = 1;
        
        dirstatus = apr_dir_open (&this_dir, dir_path, loop_pool);
        if (dirstatus != APR_SUCCESS) {
            retcode = dirstatus;
        }
        else {
            for (dirstatus = apr_dir_read (&this_entry, flags, this_dir);
                 dirstatus == APR_SUCCESS;
                 dirstatus = apr_dir_read (&this_entry, flags, this_dir)) {


                if (this_entry.filetype == APR_DIR) {
                    if (this_entry.fname[0] == '.' 
                        && (this_entry.fname[1] == '\0'
                            || (this_entry.fname[1] == '.' 
                                && this_entry.fname[2] == '\0')))
                        continue;

                    *(const char**)apr_array_push (arr) = apr_pstrdup (array_pool,
                                                                       this_entry.fname);
                    dir_is_empty = 0;
                    break;
                }
                else {
                    file_path = apr_pstrcat (loop_pool, dir_path, PATH_SEPARATOR, 
                                            this_entry.fname, NULL);
                    status = apr_file_remove (file_path, loop_pool);
                    if ((status != APR_SUCCESS) && (retcode != APR_SUCCESS)) {
                        retcode = status;
                    }
                }
            }

            if (! (APR_STATUS_IS_ENOENT (dirstatus)) && (retcode != APR_SUCCESS)) {
                retcode = dirstatus;
            }
            dirstatus = apr_dir_close (this_dir);
            if ((dirstatus != APR_SUCCESS) && (retcode != APR_SUCCESS)) {
                retcode = dirstatus;
            }
        }
        
        if (dir_is_empty) {
            dirstatus = apr_dir_remove (dir_path, loop_pool);
            if ((dirstatus != APR_SUCCESS) && (retcode != APR_SUCCESS)) {
                retcode = dirstatus;
            }
            apr_array_pop (arr);
        }        
    }
    
    apr_pool_destroy (array_pool);
    apr_pool_destroy (loop_pool);
    return retcode;
    
    
}

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to