Re: svn commit: r33826 - in trunk/subversion: include libsvn_fs_fs libsvn_subr libsvn_wc

[Greg Stein <gs...@gmail.com>]

On Tue, Oct 21, 2008 at 1:16 PM,  <hw...@tigris.org> wrote:
>...
\> +++ trunk/subversion/include/svn_io.h   Tue Oct 21 13:16:27 2008
    (r33826)
> @@ -1391,6 +1391,22 @@ svn_io_file_write_full(apr_file_t *file,
>                        apr_size_t *bytes_written,
>                        apr_pool_t *pool);
>
> +/**
> + * Open a unique file in @a dirpath, and write @a nbytes from @a buf to
> + * the file before closing it.  If @a tmp_path is not @c NULL, return the
> + * name of the newly created file in @a *tmp_path, allocated in @a pool.

What?! Okay... so let's see. If tmp_path *IS* NULL, then I won't know
where it put my data. Did I get that right?

:-P

> +++ trunk/subversion/libsvn_subr/io.c   Tue Oct 21 13:16:27 2008        (r33826)
> @@ -2719,6 +2719,24 @@ svn_io_file_write_full(apr_file_t *file,
>
>
>  svn_error_t *
> +svn_io_write_unique(const char **tmp_path,
> +                    const char *dirpath,
> +                    const void *buf,
> +                    apr_size_t nbytes,
> +                    svn_io_file_del_t delete_when,
> +                    apr_pool_t *pool)
> +{
> +  apr_file_t *new_file;
> +
> +  SVN_ERR(svn_io_open_unique_file2(&new_file, tmp_path, dirpath, ".tmp",
> +                                   delete_when, pool));
> +  SVN_ERR(svn_io_file_write_full(new_file, buf, nbytes, NULL, pool));
> +  SVN_ERR(svn_io_file_flush_to_disk(new_file, pool));
> +  return svn_io_file_close(new_file, pool);

Why the flush? You close the file on the next line. By definition, all
writes will be flushed.

Cheers,
-g

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r33826 - in trunk/subversion: include libsvn_fs_fs libsvn_subr libsvn_wc

["Hyrum K. Wright" <hy...@mail.utexas.edu>]

Greg Stein wrote:
> On Tue, Oct 21, 2008 at 1:16 PM,  <hw...@tigris.org> wrote:
>> ...
> \> +++ trunk/subversion/include/svn_io.h   Tue Oct 21 13:16:27 2008
>     (r33826)
>> @@ -1391,6 +1391,22 @@ svn_io_file_write_full(apr_file_t *file,
>>                        apr_size_t *bytes_written,
>>                        apr_pool_t *pool);
>>
>> +/**
>> + * Open a unique file in @a dirpath, and write @a nbytes from @a buf to
>> + * the file before closing it.  If @a tmp_path is not @c NULL, return the
>> + * name of the newly created file in @a *tmp_path, allocated in @a pool.
> 
> What?! Okay... so let's see. If tmp_path *IS* NULL, then I won't know
> where it put my data. Did I get that right?
> 
> :-P

Heh.  Good point.

>> +++ trunk/subversion/libsvn_subr/io.c   Tue Oct 21 13:16:27 2008        (r33826)
>> @@ -2719,6 +2719,24 @@ svn_io_file_write_full(apr_file_t *file,
>>
>>
>>  svn_error_t *
>> +svn_io_write_unique(const char **tmp_path,
>> +                    const char *dirpath,
>> +                    const void *buf,
>> +                    apr_size_t nbytes,
>> +                    svn_io_file_del_t delete_when,
>> +                    apr_pool_t *pool)
>> +{
>> +  apr_file_t *new_file;
>> +
>> +  SVN_ERR(svn_io_open_unique_file2(&new_file, tmp_path, dirpath, ".tmp",
>> +                                   delete_when, pool));
>> +  SVN_ERR(svn_io_file_write_full(new_file, buf, nbytes, NULL, pool));
>> +  SVN_ERR(svn_io_file_flush_to_disk(new_file, pool));
>> +  return svn_io_file_close(new_file, pool);
> 
> Why the flush? You close the file on the next line. By definition, all
> writes will be flushed.

Some of the code that was removed used an explicit flush before closing the
file.  I'm unsure if all OSs guarantee a flush before closing the file, but am
happy to remove this call if it proves redundant.

-Hyrum