You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Ph. Marek" <ph...@bmlv.gv.at> on 2004/12/23 11:00:48 UTC
[PATCH] [BUG] fsfs truncates rev-props - bug on full fs
Hello everyone,
I found a (IMO severe) bug in fsfs: it opens the rev-prop-files with
APR_TRUNCATE. This leads to deleted revision-properties, if the file system
is full.
The correct way would be to write a temporary file and rename that afterwards
(as you know already :-)
Eg:
svn ps --revprop -r xx svn:log
deletes all revision-properties on a full filesystem.
The culprit is (for 1.1.2) in subversion/libsvn_fs_fs/fs_fs.c line 1008
function svn_fs_fs__set_revision_proplist.
See below for a patch.
But as I received no comments on my question about correct usage of
svn_io_open_unique_file this will be more of a proof-of-concept.
Shall I write an issue too?
Regards,
Phil
PS: probably I won't be reading this mailing list until Jan 4.
[[[
* subversion/libsvn_fs_fs/fs_fs.c.orig
(svn_fs_fs__set_revision_proplist):
Create a temporary file, and rename that later.
Avoids deleting revision properties on a full filesystem.
]]]
diff -up subversion/libsvn_fs_fs/fs_fs.c.orig subversion/libsvn_fs_fs/fs_fs.c
--- subversion/libsvn_fs_fs/fs_fs.c.orig 2004-12-23 07:45:38.000000000
+0100
+++ subversion/libsvn_fs_fs/fs_fs.c 2004-12-23 11:54:21.000000000 +0100
@@ -1011,15 +1011,27 @@ svn_fs_fs__set_revision_proplist (svn_fs
apr_pool_t *pool)
{
apr_file_t *revprop_file;
+ char *tmp_file, *target;
+
+ /* We set delete_on_close to true.
+ If the operation succeeds, the file is renamed before
+ getting deleted.
+
+ If we fail, it will be cleaned up with the pool. */
+ SVN_ERR(svn_io_open_unique_file (&revprop_file, &tmp_file,
+ path_revprops (fs, rev, pool),
+ ".tmp",
+ TRUE, pool));
- SVN_ERR (svn_io_file_open (&revprop_file, path_revprops (fs, rev, pool),
- APR_WRITE | APR_TRUNCATE | APR_CREATE,
- APR_OS_DEFAULT, pool));
-
SVN_ERR (svn_hash_write (proplist, revprop_file, pool));
+ SVN_ERR (svn_io_file_flush_to_disk (revprop_file, pool));
+
+ revprop_file->flags &= ~APR_DELONCLOSE;
SVN_ERR (svn_io_file_close (revprop_file, pool));
-
+
+ SVN_ERR (move_into_place (tmp_file, target, target, pool));
+
return SVN_NO_ERROR;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [BUG] fsfs truncates rev-props - bug on full fs
Posted by "Ph. Marek" <ph...@bmlv.gv.at>.
> Can you file this as a new issue, with patch attached to the issue?
It's 2193:
http://subversion.tigris.org/issues/show_bug.cgi?id=2193
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] [BUG] fsfs truncates rev-props - bug on full fs
Posted by Ben Collins-Sussman <su...@collab.net>.
Can you file this as a new issue, with patch attached to the issue?
On Dec 23, 2004, at 5:00 AM, Ph. Marek wrote:
> Hello everyone,
>
>
> I found a (IMO severe) bug in fsfs: it opens the rev-prop-files with
> APR_TRUNCATE. This leads to deleted revision-properties, if the file
> system
> is full.
>
> The correct way would be to write a temporary file and rename that
> afterwards
> (as you know already :-)
>
>
> Eg:
> svn ps --revprop -r xx svn:log
> deletes all revision-properties on a full filesystem.
>
>
> The culprit is (for 1.1.2) in subversion/libsvn_fs_fs/fs_fs.c line 1008
> function svn_fs_fs__set_revision_proplist.
> See below for a patch.
> But as I received no comments on my question about correct usage of
> svn_io_open_unique_file this will be more of a proof-of-concept.
>
>
> Shall I write an issue too?
>
>
> Regards,
>
> Phil
>
>
> PS: probably I won't be reading this mailing list until Jan 4.
>
>
> [[[
>
> * subversion/libsvn_fs_fs/fs_fs.c.orig
> (svn_fs_fs__set_revision_proplist):
> Create a temporary file, and rename that later.
> Avoids deleting revision properties on a full filesystem.
>
> ]]]
>
>
> diff -up subversion/libsvn_fs_fs/fs_fs.c.orig
> subversion/libsvn_fs_fs/fs_fs.c
> --- subversion/libsvn_fs_fs/fs_fs.c.orig 2004-12-23
> 07:45:38.000000000
> +0100
> +++ subversion/libsvn_fs_fs/fs_fs.c 2004-12-23 11:54:21.000000000
> +0100
> @@ -1011,15 +1011,27 @@ svn_fs_fs__set_revision_proplist (svn_fs
> apr_pool_t *pool)
> {
> apr_file_t *revprop_file;
> + char *tmp_file, *target;
> +
> + /* We set delete_on_close to true.
> + If the operation succeeds, the file is renamed before
> + getting deleted.
> +
> + If we fail, it will be cleaned up with the pool. */
> + SVN_ERR(svn_io_open_unique_file (&revprop_file, &tmp_file,
> + path_revprops (fs, rev, pool),
> + ".tmp",
> + TRUE, pool));
>
> - SVN_ERR (svn_io_file_open (&revprop_file, path_revprops (fs, rev,
> pool),
> - APR_WRITE | APR_TRUNCATE | APR_CREATE,
> - APR_OS_DEFAULT, pool));
> -
> SVN_ERR (svn_hash_write (proplist, revprop_file, pool));
>
> + SVN_ERR (svn_io_file_flush_to_disk (revprop_file, pool));
> +
> + revprop_file->flags &= ~APR_DELONCLOSE;
> SVN_ERR (svn_io_file_close (revprop_file, pool));
> -
> +
> + SVN_ERR (move_into_place (tmp_file, target, target, pool));
> +
> return SVN_NO_ERROR;
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org