You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2011/10/19 17:49:49 UTC
svn commit: r1186283 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Author: hwright
Date: Wed Oct 19 15:49:49 2011
New Revision: 1186283
URL: http://svn.apache.org/viewvc?rev=1186283&view=rev
Log:
No functional change.
* subversion/libsvn_fs_fs/fs_fs.c
(set_revision_proplist): Remove a unneeded if-statement, and a superfluous
return statement.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1186283&r1=1186282&r2=1186283&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Oct 19 15:49:49 2011
@@ -2905,32 +2905,29 @@ set_revision_proplist(svn_fs_t *fs,
apr_hash_t *proplist,
apr_pool_t *pool)
{
+ const char *final_path;
+ const char *tmp_path;
+ const char *perms_reference;
+ svn_stream_t *stream;
+
SVN_ERR(ensure_revision_exists(fs, rev, pool));
- if (1)
- {
- const char *final_path = path_revprops(fs, rev, pool);
- const char *tmp_path;
- const char *perms_reference;
- svn_stream_t *stream;
-
- /* ### do we have a directory sitting around already? we really shouldn't
- ### have to get the dirname here. */
- SVN_ERR(svn_stream_open_unique(&stream, &tmp_path,
- svn_dirent_dirname(final_path, pool),
- svn_io_file_del_none, pool, pool));
- SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
- SVN_ERR(svn_stream_close(stream));
+ final_path = path_revprops(fs, rev, pool);
- /* We use the rev file of this revision as the perms reference,
- because when setting revprops for the first time, the revprop
- file won't exist and therefore can't serve as its own reference.
- (Whereas the rev file should already exist at this point.) */
- SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
- SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
+ /* ### do we have a directory sitting around already? we really shouldn't
+ ### have to get the dirname here. */
+ SVN_ERR(svn_stream_open_unique(&stream, &tmp_path,
+ svn_dirent_dirname(final_path, pool),
+ svn_io_file_del_none, pool, pool));
+ SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
+ SVN_ERR(svn_stream_close(stream));
- return SVN_NO_ERROR;
- }
+ /* We use the rev file of this revision as the perms reference,
+ because when setting revprops for the first time, the revprop
+ file won't exist and therefore can't serve as its own reference.
+ (Whereas the rev file should already exist at this point.) */
+ SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
+ SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
return SVN_NO_ERROR;
}
Re: svn commit: r1186283 -
/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Wed, Oct 19, 2011 at 12:54:59 -0500:
> On Wed, Oct 19, 2011 at 12:44 PM, Stefan Sperling <st...@elego.de> wrote:
> > On Wed, Oct 19, 2011 at 07:27:50PM +0200, Daniel Shahaf wrote:
> >> Please revert. The if (1) is there to make merging the revprop-packing
> >> branch easier.
>
> I'm happy to revert, but as the current state appears to be temporary,
> what's the plan to change it / make it permanent?
>
On the revprop-packing branch those if (1)'s are all 'if is_packed_revprop()'.
The if (1)'s are on trunk because of the revprop-packing-via-sqlite code
that used to live on trunk: when pulling the feature, I made them into
'if (1)' rather than remove them entirely, to make merging easier in the
future.
> >> If you don't like the if (1) line, feel free to delete *just that line*
> >> and leave the {} block intact...
>
> It wasn't that line, actually, it was the duplicate return statements
> which initially led me to that function.
>
Oh. Is there a way we can write set_revision_proplist() on trunk such
that your compiler or analyser doesn't complain about it?
> > I think I also removed this once.
> > Can you please add a comment if you intend to keep it?
>
> +1
>
Done.
> -Hyrum
>
>
Thanks,
Daniel
Re: svn commit: r1186283 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Wed, Oct 19, 2011 at 12:44 PM, Stefan Sperling <st...@elego.de> wrote:
> On Wed, Oct 19, 2011 at 07:27:50PM +0200, Daniel Shahaf wrote:
>> Please revert. The if (1) is there to make merging the revprop-packing
>> branch easier.
I'm happy to revert, but as the current state appears to be temporary,
what's the plan to change it / make it permanent?
>> If you don't like the if (1) line, feel free to delete *just that line*
>> and leave the {} block intact...
It wasn't that line, actually, it was the duplicate return statements
which initially led me to that function.
> I think I also removed this once.
> Can you please add a comment if you intend to keep it?
+1
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1186283 -
/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Oct 19, 2011 at 07:27:50PM +0200, Daniel Shahaf wrote:
> Please revert. The if (1) is there to make merging the revprop-packing
> branch easier.
>
> If you don't like the if (1) line, feel free to delete *just that line*
> and leave the {} block intact...
I think I also removed this once.
Can you please add a comment if you intend to keep it?
Re: svn commit: r1186283 -
/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Please revert. The if (1) is there to make merging the revprop-packing
branch easier.
If you don't like the if (1) line, feel free to delete *just that line*
and leave the {} block intact...
hwright@apache.org wrote on Wed, Oct 19, 2011 at 15:49:49 -0000:
> Author: hwright
> Date: Wed Oct 19 15:49:49 2011
> New Revision: 1186283
>
> URL: http://svn.apache.org/viewvc?rev=1186283&view=rev
> Log:
> No functional change.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (set_revision_proplist): Remove a unneeded if-statement, and a superfluous
> return statement.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1186283&r1=1186282&r2=1186283&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Oct 19 15:49:49 2011
> @@ -2905,32 +2905,29 @@ set_revision_proplist(svn_fs_t *fs,
> apr_hash_t *proplist,
> apr_pool_t *pool)
> {
> + const char *final_path;
> + const char *tmp_path;
> + const char *perms_reference;
> + svn_stream_t *stream;
> +
> SVN_ERR(ensure_revision_exists(fs, rev, pool));
>
> - if (1)
> - {
> - const char *final_path = path_revprops(fs, rev, pool);
> - const char *tmp_path;
> - const char *perms_reference;
> - svn_stream_t *stream;
> -
> - /* ### do we have a directory sitting around already? we really shouldn't
> - ### have to get the dirname here. */
> - SVN_ERR(svn_stream_open_unique(&stream, &tmp_path,
> - svn_dirent_dirname(final_path, pool),
> - svn_io_file_del_none, pool, pool));
> - SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
> - SVN_ERR(svn_stream_close(stream));
> + final_path = path_revprops(fs, rev, pool);
>
> - /* We use the rev file of this revision as the perms reference,
> - because when setting revprops for the first time, the revprop
> - file won't exist and therefore can't serve as its own reference.
> - (Whereas the rev file should already exist at this point.) */
> - SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
> - SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
> + /* ### do we have a directory sitting around already? we really shouldn't
> + ### have to get the dirname here. */
> + SVN_ERR(svn_stream_open_unique(&stream, &tmp_path,
> + svn_dirent_dirname(final_path, pool),
> + svn_io_file_del_none, pool, pool));
> + SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
> + SVN_ERR(svn_stream_close(stream));
>
> - return SVN_NO_ERROR;
> - }
> + /* We use the rev file of this revision as the perms reference,
> + because when setting revprops for the first time, the revprop
> + file won't exist and therefore can't serve as its own reference.
> + (Whereas the rev file should already exist at this point.) */
> + SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
> + SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
>
> return SVN_NO_ERROR;
> }
>
>
Re: svn commit: r1186283 -
/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Please revert. The if (1) is there to make merging the revprop-packing
branch easier.
If you don't like the if (1) line, feel free to delete *just that line*
and leave the {} block intact...
hwright@apache.org wrote on Wed, Oct 19, 2011 at 15:49:49 -0000:
> Author: hwright
> Date: Wed Oct 19 15:49:49 2011
> New Revision: 1186283
>
> URL: http://svn.apache.org/viewvc?rev=1186283&view=rev
> Log:
> No functional change.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (set_revision_proplist): Remove a unneeded if-statement, and a superfluous
> return statement.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1186283&r1=1186282&r2=1186283&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Oct 19 15:49:49 2011
> @@ -2905,32 +2905,29 @@ set_revision_proplist(svn_fs_t *fs,
> apr_hash_t *proplist,
> apr_pool_t *pool)
> {
> + const char *final_path;
> + const char *tmp_path;
> + const char *perms_reference;
> + svn_stream_t *stream;
> +
> SVN_ERR(ensure_revision_exists(fs, rev, pool));
>
> - if (1)
> - {
> - const char *final_path = path_revprops(fs, rev, pool);
> - const char *tmp_path;
> - const char *perms_reference;
> - svn_stream_t *stream;
> -
> - /* ### do we have a directory sitting around already? we really shouldn't
> - ### have to get the dirname here. */
> - SVN_ERR(svn_stream_open_unique(&stream, &tmp_path,
> - svn_dirent_dirname(final_path, pool),
> - svn_io_file_del_none, pool, pool));
> - SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
> - SVN_ERR(svn_stream_close(stream));
> + final_path = path_revprops(fs, rev, pool);
>
> - /* We use the rev file of this revision as the perms reference,
> - because when setting revprops for the first time, the revprop
> - file won't exist and therefore can't serve as its own reference.
> - (Whereas the rev file should already exist at this point.) */
> - SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
> - SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
> + /* ### do we have a directory sitting around already? we really shouldn't
> + ### have to get the dirname here. */
> + SVN_ERR(svn_stream_open_unique(&stream, &tmp_path,
> + svn_dirent_dirname(final_path, pool),
> + svn_io_file_del_none, pool, pool));
> + SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
> + SVN_ERR(svn_stream_close(stream));
>
> - return SVN_NO_ERROR;
> - }
> + /* We use the rev file of this revision as the perms reference,
> + because when setting revprops for the first time, the revprop
> + file won't exist and therefore can't serve as its own reference.
> + (Whereas the rev file should already exist at this point.) */
> + SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
> + SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
>
> return SVN_NO_ERROR;
> }
>
>