Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Errors-To: <wrowe@rowe-clan.net>
Message-Id: <5.1.0.14.2.20020702151116.00bdc138@pop3.rowe-clan.net>
Date: Tue, 02 Jul 2002 15:13:42 -0500
To: kfogel@collab.net
From: "William A. Rowe, Jr." <wrowe@rowe-clan.net>
Subject: Re: proposal to add apr_check_dir_empty() to APR
Cc: dev@apr.apache.org, dev@subversion.tigris.org
In-Reply-To: <200207021948.g62JmP409555@newton.ch.collab.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed

At 02:48 PM 7/2/2002, Karl Fogel wrote:
>Currently, apr_check_dir_empty() is living in the Subversion source
>tree.  The implementation looks portable, though; is there any reason
>not to move this into APR?

One, the name sucks... perhaps apr_dir_is_empty()?  Keep it named with the
other apr_dir_ family members and our general schema.  We have to work hard
on consistency so we avoid symbol renames of newly created functions.

Other than that, I would _NOT_ assume all first two entries are "."/"..", but
if you make that an explicit string test, I'd be +1.

Also, APR_EGENERAL seems a bit vague, how about APR_EEXISTS?

>    apr_status_t
>    apr_check_dir_empty (const char *path,
>                         apr_pool_t *pool)
>    {
>      apr_status_t apr_err, retval;
>      apr_dir_t *dir;
>      apr_finfo_t finfo;
>
>      apr_err = apr_dir_open (&dir, path, pool);
>      if (apr_err)
>        return apr_err;
>
>      /* All systems return "." and ".." as the first two files, so read
>         past them unconditionally. */
>      apr_err = apr_dir_read (&finfo, APR_FINFO_NAME, dir);
>      if (apr_err) return apr_err;
>      apr_err = apr_dir_read (&finfo, APR_FINFO_NAME, dir);
>      if (apr_err) return apr_err;
>
>      /* Now, there should be nothing left.  If there is something left,
>         return EGENERAL. */
>      apr_err = apr_dir_read (&finfo, APR_FINFO_NAME, dir);
>      if (APR_STATUS_IS_ENOENT (apr_err))
>        retval = APR_SUCCESS;
>      else if (! apr_err)
>        retval = APR_EGENERAL;
>      else
>        retval = apr_err;
>
>      apr_err = apr_dir_close (dir);
>      if (apr_err)
>        return apr_err;
>
>      return retval;
>    }
>
>Thoughts?,
>-Karl