possible file/dir handle leaks

While trying to find out why there's a leaked file handle
(see http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=52249
for the complete thread about this.) and where it could be leaked I
stumpled across some code parts which deserve some reviewing (I think).

Just for the record: I'm not sure if those are 1. really leaks and 2. if
they
are the reason for the leak described in the thread mentioned above.

Also such leaked handles are not a problem for the command line client
since as soon as the process terminates the OS (at least newer
windows-versions do)
closes all still open filehandles owned by that process. But TSVN is
a shell extension and *never* terminates as long as the user
doesn't log off or shut down the system - so for TSVN it's a real
problem if something like this happens.

Maybe someone more familliar with the code for "svn st"
(especially the parts where a file is diffed against BASE
to check if it is modified) can find out exactly where the leak
is - I was just browsing through the code looking for leaks.

Ok, on to the code parts I found suspicious - something like
I'm accusing and now the judge has to decide if those parts
are guilty or innocent :)

libsvn_subr/io.c
static int test_tempdir(const char *temp_dir, apr_pool_t *p)
{
    apr_file_t *dummy_file;
    const char *path = apr_pstrcat(p, temp_dir, "/apr-tmp.XXXXXX", NULL);

    if (apr_file_mktemp(&dummy_file, (char *)path, 0, p) == APR_SUCCESS) {
        if (apr_file_putc('!', dummy_file) == APR_SUCCESS) {
<------------------- if the write fails, the file is not closed!
            if (apr_file_close(dummy_file) == APR_SUCCESS) {
                return 1;
            }
        }
    }
    return 0;
}

Also this function may leak - but I have to admit that
I can't think of a case in which a CreateFile could
fail on a file but an apr_stat would succeed.
(in libsvn_subr/io.c):
svn_error_t *
svn_io_open_unique_file (apr_file_t **f,
                         const char **unique_name_p,
                         const char *path,
                         const char *suffix,
                         svn_boolean_t delete_on_close,
                         apr_pool_t *pool)
{
[snip]
          /* On Win32, CreateFile failswith an "Access Denied" error
             code, rather than "File Already Exists", if the colliding
             name belongs to a directory. */
          if (APR_STATUS_IS_EACCES (apr_err))
            {
              apr_finfo_t finfo;
              apr_status_t apr_err_2 = apr_stat (&finfo, unique_name_apr,
                                                 APR_FINFO_TYPE, pool);

              if (APR_STATUS_IS_SUCCESS (apr_err_2)
                  && (finfo.filetype == APR_DIR))
                continue; <------------------ if a status could be
fetched, but it's not a directory (maybe a file?) the handle isn't closed!

              /* Else ignore apr_err_2; better to fall through and
                 return the original error. */
            }
[snip]
}

and in apr/file_io/win32/dir.c
APR_DECLARE(apr_status_t) apr_dir_read(apr_finfo_t *finfo, apr_int32_t
wanted,
                                       apr_dir_t *thedir)
{
[snip]
        else if (!FindNextFileW(thedir->dirhand, thedir->w.entry)) {
            return apr_get_os_error(); <------ the handle from
FindFirstFile() is not closed with FindClose()!
        }
[snip]
}

---
Every morning is the dawn of a new error.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org